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

Build selected build-target when starting to debug #2987

Merged
merged 4 commits into from
Feb 6, 2023

Conversation

Maddimax
Copy link
Contributor

This changes visible behavior

The following changes are proposed:

  • When launching, not only build the selected launch target, but also the selected build target if the two differ.

The purpose of this change

For some projects, you always want to build ALL. This allows to set ALL as the build target, but still launch the selected launch target.

@Maddimax
Copy link
Contributor Author

@microsoft-github-policy-service agree

const buildResult = await this.build([chosen.name]);
const defaultBuildTargets = await this.getDefaultBuildTargets();
const allTargetsToBuild = new Set(defaultBuildTargets);
allTargetsToBuild.add(chosen.name);
Copy link
Contributor

@elahehrashedi elahehrashedi Jan 31, 2023

Choose a reason for hiding this comment

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

Check if the defaultBuildTargets is equal to allTargetName. In that case, don't add the chosen.name to the list of targets.

Copy link
Member

@bobbrow bobbrow Jan 31, 2023

Choose a reason for hiding this comment

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

No, you should check if the set contains this.allTargetName. If that's the case, then you don't need to add the debug target to the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

use this.allTargetName instead of all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@elahehrashedi elahehrashedi enabled auto-merge (squash) February 6, 2023 17:57
@elahehrashedi elahehrashedi merged commit f2e51f3 into microsoft:main Feb 6, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants