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

PR for issue 1711 #2421

Closed

Conversation

vaclavHala
Copy link

I had to extract the part which does variable resolution from TargetDefinitionResolver into the .shared bundle
so it is available both in TargetDefinitionResolver but also in Mojos which deal with repository location.

Also had to make some public access to TargetDefinition.Repository implementation in TargetDefinitionFile.
This is very ugly, I feel the problem here is the Repository in TargetDefinition and TargetDefinitionFile
should in fact not be the same class as one represents the repository element as present in .target file
(potentially with unresolved variables) and the other is resolved repository with valid URI.
Changing this would be a bigger refactoring I felt would be too invasive to do here.

I did not find any test for update-target and am not sure how to write one (since it updates files in place)
but at least tested that one manually and seems to work fine.
The other affected plugin, target validation Mojo, is tested in IT along with testing the locations resolve correctly.

@vaclavHala
Copy link
Author

not sure what is up with the ECA, I already have that signed and contributed PR to this repo before #622

@vaclavHala
Copy link
Author

any chance the failed tests in CI builds are just flaky (all tests passed for me locally) or did I actually break something?

@laeubi
Copy link
Member

laeubi commented May 18, 2023

About the ECA, make sure your commit uses EXACTLY the mail you signed the ECA with, about the tests, I fear there was no build long long time ago to decide on this, the main problem would be that you probabbly need to adjust the build so it uses a jdk < 17 .

@laeubi
Copy link
Member

laeubi commented Jan 23, 2024

@vaclavHala is this still important for you?

If yes I would suggest to first create a PR to make the 2.7. branch producing snapshots again and the backport:

@laeubi laeubi closed this Jan 23, 2024
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.

2 participants