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

Importing a BOM works even when it has the same components in dep mgmt #25

Merged
merged 3 commits into from
Jun 23, 2022

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Nov 28, 2017

Does not correspond to any known bug, but it seems like a potentially unusual practice deserving of test coverage.

CC @stephenc who wondered whether this actually works

@@ -654,6 +654,7 @@ public static Test suite()
suite.addTestSuite( MavenITmng0249ResolveDepsFromReactorTest.class );
suite.addTestSuite( MavenITmng0187CollectedProjectsTest.class );
suite.addTestSuite( MavenITmng0095ReactorFailureBehaviorTest.class );
suite.addTestSuite( MavenIT0199CyclicImportScopeTest.class );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was unclear what the naming scheme was here, so I just made something up.

Verifier verifier = newVerifier(new File(testDir.getAbsolutePath(), module).getAbsolutePath());
verifier.setAutoclean(false);
verifier.deleteDirectory("target");
verifier.executeGoal("install");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested iteratively using

rm -rfv repo/test
mvn clean test -f core-it-suite -Prun-its -Dmaven.repo.local=`pwd`/repo -Dtest=org.apache.maven.it.MavenIT0199CyclicImportScopeTest

@ghost
Copy link

ghost commented Jan 11, 2018

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@jtnord
Copy link

jtnord commented Apr 2, 2020

@jglick was there a Jira filed for this - I think this has got lost on the Apache side?

//cc @aheritier

@aheritier
Copy link
Member

Effectively it's often hard to monitor all PRs in all Maven related repo
I am not sure if someone saw it ( cc @hboutemy @rfscholte ) without a Jira task in the ASF side

@jglick
Copy link
Contributor Author

jglick commented Apr 2, 2020

I do not recall any Apache JIRA issue.

@rfscholte
Copy link
Contributor

I've missed a lot of github activity in the period there were read only mirrors. Having 2 overlapping systems (github vs jira) generates a lot of email traffic, I'm not able to keep up and respond to it.

@jglick
Copy link
Contributor Author

jglick commented Jun 10, 2022

@gnodet @cstamas et al., is there someone to review this sort of thing?

@olamy
Copy link
Member

olamy commented Jun 16, 2022

this looks to be a good idea to have an IT to ensure this will keep working and as is being a documented feature :)
Currently this repo doesn't have any build by itself as it's running after a build maven core so we do not have any way to validate changes.
So I have created this #172 to be able to validate such changes.
once #172 is merged if you could rebase that would be great.
thanks

@olamy
Copy link
Member

olamy commented Jun 20, 2022

@jglick the PR has been merged. Can you update your branch? Then I will merge it. Thanks

@jglick
Copy link
Contributor Author

jglick commented Jun 22, 2022

2022-06-22T18:08:34.5840918Z 0199 CyclicImportScope.it0199().............................. OK (2.2 s)

@olamy olamy merged commit 4f80553 into apache:master Jun 23, 2022
@jglick jglick deleted the cyclic-import-scope branch June 23, 2022 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants