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

Cleanup and added features #8

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Falco20019
Copy link
Collaborator

  • Cleanup of some constant usings
  • Unification of baseUrl usage
  • Extraction of report generation for others to use
  • Addition of DistroessedExceptional for checking the json file against a common logic (to be extracted into a separate JSON in the future)

@richlander
Copy link
Owner

This will take me a few days to get to. Just an ACK that I have seen and will look look at this.

@richlander richlander closed this Aug 30, 2024
@richlander richlander reopened this Aug 30, 2024
@richlander
Copy link
Owner

Closing and re-opening to get the PR to run the new CI.

@@ -100,9 +104,15 @@ public record SdkComponent(
[property: Description("The version of Visual Studio that includes this component version.")]
string VSVersion,

[property: Description("The version of Visual Studio on MacOS that includes this component version.")]
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not in favor of adding these. At this point, these properties are pure legacy. What is the motivation for keeping them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not intentionally add those. I only rebased what I already had onto your latest state and it seems they were re-added by that. I will remove them once I‘m back in office mid next week. Thanks for noticing!

Copy link
Owner

Choose a reason for hiding this comment

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

Got it. No worries.

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