Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

2.7: Changes to Auto-configuration #156#182

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@ijusti
Copy link
Contributor

No description provided.

@pivotal-cla
Copy link

@ijusti Please sign theContributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See theFAQ for frequently asked questions.

@pivotal-cla
Copy link

@ijusti Thank you for signing theContributor License Agreement!

@fabapp2
Copy link
Contributor

Hi@ijusti!
I've been on PTO thus the long delay.
Thanks a lot for your contribution and the many beautifications and fixes. Very much appreciated! 🙏
I had a few comments regarding the operation on files and would ask you to provide at least a test for theAction itself.
If you could resolve these comments I'll happily merge your PR.
Thanks again!

}

@Override
publicbooleanevaluate(ProjectContextcontext) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use e.g.PathPatternMatchingProjectResourceFinder instead?
This would also remove the dependency fromCondition toAction and remove the assumption that the resources in theProjectContext exist as files.

return !context.search(new PathPatternMatchingProjectResourceFinder("**/src/main/resources/META-INF/spring.factories")).isEmpty();

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

i need to check that near spring.factories no new file exists

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, that's a good idea. What about something like this?

return !context.search(new PathPatternMatchingProjectResourceFinder("**/src/main/resources/META-INF/spring.factories")).isEmpty() && context.search(new PathPatternMatchingProjectResourceFinder("**/src/main/resources/META-INF/org.springframework.boot.autoconfigure.AutoConfiguration.imports")).isEmpty()

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

this good, and I tried to make this way? but there is no spring.factories in context

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

something like this, that mean that there is no new imports near spring.factories

return context.search(new PathPatternMatchingProjectResourceFinder("**/src/main/resources/META-INF/spring.factories")).stream().anyMatch(resource-> context.search(new PathPatternMatchingProjectResourceFinder("resource.path.parent +"org.springframework.boot.autoconfigure.AutoConfiguration.imports")).isEmpty())

fabapp2 reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I got your valid point here.
I am thinking that if we replace all old with new factories in one go and remove the old ones (or move withMoveFilesAction), there should not be both (old and new) in one project. If this is "guaranteed" we can skip the second part of the check?

You could replaceresource.path.parent withresource.getAbsoluetPath().getParent().resolve("org.springframework.boot.autoconfigure.AutoConfiguration.imports") and then useAbsolutePathResourceFinder with this path.

importstaticjava.nio.file.StandardOpenOption.WRITE;
importstaticjava.util.function.Predicate.not;

publicclassCreateAutoconfigurationActionextendsAbstractAction {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please provide a test for this class.
You can create a syntheticProjectContext withTestProjectContext helper class.
Run the Action against the createdProjectConetxt and verify the result.
You can have a look at existing tests for examples.

ijusti reacted with thumbs up emoji
@fabapp2fabapp2 linked an issueJul 7, 2022 that may beclosed by this pull request
}

@Override
publicbooleanevaluate(ProjectContextcontext) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, that's a good idea. What about something like this?

return !context.search(new PathPatternMatchingProjectResourceFinder("**/src/main/resources/META-INF/spring.factories")).isEmpty() && context.search(new PathPatternMatchingProjectResourceFinder("**/src/main/resources/META-INF/org.springframework.boot.autoconfigure.AutoConfiguration.imports")).isEmpty()

@fabapp2
Copy link
Contributor

Hi@ijusti !
Could you provide the requested changes?
I am sorry if I am pushy here but we'll need this issue (very) soon.
It's no problem if you don't have time, I'd merge what you did and add the requested changes myself then (crediting you as author).
Let me know what works for you?

ijusti reacted with thumbs up emojifabapp2 reacted with hooray emoji

@ijustiijustiforce-pushed thesb27-autoconfiguration branch from9a0d8bc to9a81c94CompareJuly 20, 2022 15:24
@ijustiijustiforce-pushed thesb27-autoconfiguration branch from9a81c94 to8413ea1CompareJuly 20, 2022 15:51
@fabapp2fabapp2 merged commit6bce7ae intospring-projects-experimental:mainJul 21, 2022
@fabapp2fabapp2 added type: enhancementNew feature or request upgrade:boot labelsSep 26, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@fabapp2fabapp2fabapp2 requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type: enhancementNew feature or request

Projects

None yet

Milestone

v0.12.0

Development

Successfully merging this pull request may close these issues.

2.7: Changes to Auto-configuration

3 participants

@ijusti@pivotal-cla@fabapp2

[8]ページ先頭

©2009-2025 Movatter.jp