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

Simplify the API baseline target #42

Merged
merged 1 commit into from Aug 22, 2022

Conversation

merks
Copy link
Contributor

@merks merks commented Aug 14, 2022

Also simplify the corresponding API baseline targlet in the setup

#39

@merks
Copy link
Contributor Author

merks commented Aug 14, 2022

@jonahgraham @ruspl-afed

Guys, I submitted the changes as PR. Then I can see how the API baseline is used in the build and you can more easily review this in your local workspace...

Copy link
Member

@ruspl-afed ruspl-afed left a comment

Choose a reason for hiding this comment

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

I greet this change as it reduces support effort. Only one minor comment

<unit id="org.eclipse.tm.terminal.connector.cdtserial.feature.feature.group" version="0.0.0"/>
<unit id="org.eclipse.tm.terminal.connector.remote.feature.feature.group" version="0.0.0"/>
<unit id="org.eclipse.tm.terminal.feature.feature.group" version="0.0.0"/>
<repository location="https://download.eclipse.org/tools/cdt/releases/latest"/>
Copy link
Member

Choose a reason for hiding this comment

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

Usually we have repository at the top and then we sort locations by repository::location

@merks
Copy link
Contributor Author

merks commented Aug 14, 2022

I kind of doubt the test failures can be caused the API baseline change...

Now that I look closely, I don't think the baseline target is even used. The actual Maven build uses this which is out of date too:

		<comparator.repo>https://download.eclipse.org/tools/cdt/releases/10.6/cdt-10.6.0/</comparator.repo>

Also simplify the corresponding API baseline targlet in the setup

eclipse-cdt#39
@merks
Copy link
Contributor Author

merks commented Aug 14, 2022

Given the cdt-baseline.target is not used in the build, but rather is only used for a manual setup, reducing maintenance effort would definitely be a good thing...

@jonahgraham
Copy link
Member

I kind of doubt the test failures can be caused the API baseline change...

Now that I look closely, I don't think the baseline target is even used. The actual Maven build uses this which is out of date too:

		<comparator.repo>https://download.eclipse.org/tools/cdt/releases/10.6/cdt-10.6.0/</comparator.repo>

That is the compare and replace repo. Separate from api baseline. As mentioned in the other comment I just made that eclipse-cdt/cdt-infra#59 needs to be done for next version and currently main hasn’t been prepped for post 10.7 yet.

BTW main branch will be 11.0 next, that is a major release of CDT is upcoming.

@jonahgraham
Copy link
Member

I’m glad that you (Ed) are simplifying and that Alexander has time to review these changes. I will happily review anything left to review later.

@jonahgraham jonahgraham merged commit ccdd9b2 into eclipse-cdt:main Aug 22, 2022
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.

None yet

3 participants