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

Improved GCCToolChain.stripCommand() #374

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

ewaterlander
Copy link
Contributor

GCCToolChain.stripCommand() assumed that all resources are at the end of the command, like in the old version of
GCCToolChain.getResourcesFromCommand() which was fixed in PR #311 (see commit a89ce59). Now stripCommand() is in line with getResourcesFromCommand().

GCCToolChain.stripCommand() assumed that all resources are at the end
of the command, like in the old version of
GCCToolChain.getResourcesFromCommand() which was fixed in PR eclipse-cdt#311 (see
commit a89ce59). Now stripCommand() is in line with
getResourcesFromCommand().
@jonahgraham
Copy link
Member

The failing test is flaky and covered by #194

Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

I think this is fine - AFAICT there are no existing tests for any of this functionality so it is hard to verify if anything regresses because of this change.

If you want to reduce the chance of future regressions adding a unit test or two would be very welcome.

@jonahgraham jonahgraham merged commit f40f957 into eclipse-cdt:main Apr 21, 2023
@jonahgraham jonahgraham added this to the 11.2.0 milestone Apr 21, 2023
@jonahgraham jonahgraham added editor The CDT C/C++, Assembly, Makefile and other editors provided by CDT build Build components of CDT, anything to do with running the compiler, using Make, CMake, or any builder and removed editor The CDT C/C++, Assembly, Makefile and other editors provided by CDT labels Apr 21, 2023
@ewaterlander ewaterlander deleted the stripCommand branch April 24, 2023 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build components of CDT, anything to do with running the compiler, using Make, CMake, or any builder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants