Skip to content

Reorganise e2e samples #642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Tracked by #637
jvalkeal opened this issue Jan 26, 2023 · 0 comments
Closed
Tracked by #637

Reorganise e2e samples #642

jvalkeal opened this issue Jan 26, 2023 · 0 comments
Labels
branch/3.0.x Issue for a branch for/backport For backporting
Milestone

Comments

@jvalkeal
Copy link
Contributor

Before we start to add anything related to new annotation model #637, it'd be good if we move legacy annotation commands and registration based commands in their own inner classes.

For example:

public class RequiredValueCommands {

	private final static String REQUIRED_VALUE = "required-value";
	private final static String ARG1_DESC = "Desc arg1";
	private final static String ARG1_NAME = "arg1";

	@ShellComponent
	public static class RequiredValueCommandsLegacyAnnotation extends BaseE2ECommands {

		@ShellMethod(key = LEGACY_ANNO + REQUIRED_VALUE, group = GROUP)
		public String testRequiredValueLegacyAnnotation(
			@ShellOption(help = ARG1_DESC)
			String arg1
		) {
			return "Hello " + arg1;
		}
	}

	@Component
	public static class RequiredValueCommandsRegistration extends BaseE2ECommands {

		@Bean
		public CommandRegistration testRequiredValueRegistration(CommandRegistration.BuilderSupplier builder) {
			return builder.get()
				.command(REG, REQUIRED_VALUE)
				.group(GROUP)
				.withOption()
					.longNames(ARG1_NAME)
					.description(ARG1_DESC)
					.required()
					.and()
				.withTarget()
					.function(ctx -> {
						String arg1 = ctx.getOptionValue(ARG1_NAME);
						return "Hello " + arg1;
					})
					.and()
				.build();
		}
	}
}

This way when we start to add @Command based commands we can have new inner class as well. Not doing this will cause a rebase hell with maintenance branches.

@jvalkeal jvalkeal mentioned this issue Jan 26, 2023
7 tasks
@jvalkeal jvalkeal added this to the 3.1.0-M1 milestone Jan 26, 2023
@jvalkeal jvalkeal added for/backport For backporting branch/3.0.x Issue for a branch labels Jan 26, 2023
jvalkeal added a commit that referenced this issue Jan 26, 2023
- Backport #642
- Fixes #643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/3.0.x Issue for a branch for/backport For backporting
Projects
None yet
Development

No branches or pull requests

1 participant