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

Add ManagedBuildManager#createConfigurationForProject() #131

Merged
merged 4 commits into from
Nov 4, 2022
Merged

Add ManagedBuildManager#createConfigurationForProject() #131

merged 4 commits into from
Nov 4, 2022

Conversation

15knots
Copy link
Contributor

@15knots 15knots commented Oct 31, 2022

This should allow ISV's to create MBS based project with a vendor-specific build-system ID without using internal API.

Signed-off-by: 15knots 11367029+15knots@users.noreply.github.com

This should allow ISV's to create MBS based project with a
vendor-specific build-system ID without using internal API.

Signed-off-by: 15knots <11367029+15knots@users.noreply.github.com>
@15knots 15knots requested a review from jonahgraham October 31, 2022 18:41
@15knots
Copy link
Contributor Author

15knots commented Oct 31, 2022

Sorry, forgat the N&N.... Coming soon.

Signed-off-by: 15knots <11367029+15knots@users.noreply.github.com>
Signed-off-by: 15knots <11367029+15knots@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Oct 31, 2022

Test Results

     602 files       602 suites   18m 49s ⏱️
10 183 tests 10 162 ✔️ 21 💤 0
10 146 runs  10 125 ✔️ 21 💤 0

Results for commit 5474486.

♻️ This comment has been updated with latest results.

@15knots 15knots requested a review from mbooth101 November 2, 2022 20:36

IConfiguration config = ManagedBuildManager.createConfigurationForProject(des, mProj, cf,
ManagedBuildManager.CFG_DATA_PROVIDER_ID);
// TODO check config.exportArtifactInfo();
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this call to exportArtifactInfo() was moved to createConfigurationForProject()

What is TODO here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to delete that. It is a reminder to check whether the refactored code should call 'config.exportArtifactInfo();'.

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.

LGTM - Thanks for recreating a clean commit. Thanks @mbooth101 for doing a review too.

Signed-off-by: 15knots <11367029+15knots@users.noreply.github.com>
@15knots 15knots merged commit 369ddea into eclipse-cdt:main Nov 4, 2022
@jonahgraham jonahgraham added this to the 11.0.0 milestone Nov 4, 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.

3 participants