Skip to content
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

Springboot artifacts fix #5352

Merged
merged 2 commits into from
Jan 27, 2023
Merged

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Jan 23, 2023

There's a springboot module on the Plugin portal, but that one does not handle native-image compilation. Longer term, I'd like to create a dedicated native-image supporting module(s) and move the gradle+maven plugin support there. But for NBLS immediate release and NB17 release, I'd like to include a very trivial springboot support - Micronaut module is a good enterprise cluster candidate to make homeplace for it.

The PR adds ability to run native compilation from the NetBeans IDE based on springboot plugin support. In addition, springboot projects are configured differently to micronaut projects - the changes in MicronautPackagingArtifactsImpl make the impl more compatible with various setups, as it now reacts on the plugin's goal regardless of whether the plugin is bound to the buildchain.

The maven module part is a necessary subtle API change - the friend API did allow to create RunConfigs for an explicit goal, but not for an (abstract) project action which is needed here, and uses possible user's customizations for the action (and therefore the goal). It fits well to the RunUtils API function set.

I'd like to include this PR into NB17 release.

@sdedic sdedic added LSP [ci] enable Language Server Protocol tests Maven [ci] enable "build tools" tests VSCode Extension [ci] enable VSCode Extension tests Micronaut [ci] enable enterprise job labels Jan 23, 2023
@sdedic sdedic added this to the NB17 milestone Jan 23, 2023
@sdedic sdedic self-assigned this Jan 23, 2023
dbalek
dbalek previously approved these changes Jan 24, 2023
Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine as a temporary solution.

Copy link
Member

@neilcsmith-net neilcsmith-net left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current state, -1 for NB17. Haven't looked at the details of the implementation, but if this is to be included in NB17 it needs to be based on delivery and all spec version changes removed.

I also don't see the justification for inclusion in NB17.

@MartinBalin
Copy link
Contributor

I agree with what Neils says. But, without this fix which is needed for some of the GraalVM+SpringBoot support in VSNetBeans we will need to either diverge VSNetBeans code from NetBeans code or do a hacky solution elsewhere... I propose to do this and then fix it properly in master after NB17?

@neilcsmith-net
Copy link
Member

@MartinBalin nothing stopping the changes going into delivery and NB17 if you want them there, as long as the PR is updated to allow merging to delivery.

@sdedic assertion above about spec versions is wrong. The Maven module is incorrectly configured here as far as I can see (comment inline with code). It is certainly skipped when spec versions get incremented. So we won't get a merge conflict with master, but should fix it I think? And once delivery is synced, increment again?

@sdedic sdedic changed the base branch from master to delivery January 26, 2023 12:52
@sdedic sdedic dismissed dbalek’s stale review January 26, 2023 12:52

The base branch was changed.

@sdedic
Copy link
Member Author

sdedic commented Jan 26, 2023

Rebased onto delivery. Maven's spec version changed to use = and to 2.157.0

@neilcsmith-net neilcsmith-net dismissed their stale review January 26, 2023 13:38

Concerns addressed.

@neilcsmith-net
Copy link
Member

Thanks! That looks OK to me from perspective of merging to delivery.

I'll leave @MartinBalin to review in full and handle merging.

@MartinBalin MartinBalin merged commit ddf51ad into apache:delivery Jan 27, 2023
MartinBalin pushed a commit that referenced this pull request Jan 27, 2023
* API to create RunConfig for a project-provided action.

* Trivial SpringBoot native-image support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests Maven [ci] enable "build tools" tests Micronaut [ci] enable enterprise job VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants