Skip to content

C#: Tool status page support #12217

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.

Sign up for GitHub

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

Jump to bottom
Merged
mbg merged 40 commits into from
Mar 2, 2023
Merged

C#: Tool status page support #12217

mbg merged 40 commits into from
Mar 2, 2023

Conversation

@mbg
Copy link
Member

@mbg mbg commented Feb 16, 2023

This PR adds initial support for the tool status page to the C# autobuilder.

mbg added 16 commits February 16, 2023 14:16
@mbg
Refactor env var code in Autobuilder class
fb6c27b
@mbg
Read ..._DIAGNOSTIC_DIR variable
fce9cb0
@mbg
Add initial code for diagnostic messages
4e7c39a
@mbg
Add basic reporting of a general autobuild failure
f4c4871
@mbg
BuildCommandAutoRule: expose more information
5963501 
We expose the list of candidate script paths and the chosen script path
so that we can inspect them for diagnostics purposes.
@mbg
Add diagnostics for BuildCommandAutoRule
7b5e19d
@mbg
Add no projects/solutions diagnostic
219b232
@mbg
Fixup: We => CodeQL
c5a2cfc
@mbg
Support asynchronous stdout/stderr processing
9992491
@mbg
Change logic for autobuild failures
9865c50 
This is to account for multiple attempted rules that failed
@mbg
Set DiagnosticMessage defaults
61ff4c7 
Refactor `GetDiagnosticSource` into `MakeDiagnostic`
which sets the defaults.
@mbg
Report projects incompatible with .NET Core
f68c529
@mbg
Track which projects/solutions fail to build
eda33fc
@mbg
Report .NET Core & MSBuild failures
c55281a
@mbg
Fixup: better error message for no-projects-or-solutions
7e48084
@mbg
Detect missing Xamarin SDKs
5537d79
@mbg mbg self-assigned this Feb 16, 2023
@github-actions github-actions bot added the C# label Feb 16, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 16, 2023
View reviewed changes
@github-actions github-actions bot added the C++ label Feb 16, 2023
hvitved
hvitved reviewed Feb 17, 2023
View reviewed changes
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Looks very plausible to me. A few trivial comments.

@michaelnebel michaelnebel self-requested a review February 22, 2023 08:16
michaelnebel
michaelnebel reviewed Feb 22, 2023
View reviewed changes
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

Looks like this is going great!
Looking forward to seeing it in action :-)

@mbg mbg marked this pull request as ready for review February 28, 2023 15:23
@mbg mbg requested review from a team as code owners February 28, 2023 15:23
@mbg mbg requested review from hvitved and michaelnebel February 28, 2023 15:29
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 28, 2023
View reviewed changes
michaelnebel
michaelnebel previously approved these changes Mar 1, 2023
View reviewed changes
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@michaelnebel
Copy link
Contributor

michaelnebel commented Mar 1, 2023

Really good work!
Would it somehow be possible to merge this PR before more stuff is added? Reviewing becomes a bit easier (and faster) when the PRs are smaller. One can consider to introduce feature flags and then commit incrementally. Then it is also easier to "disable" again if there are any issues.

@mbg
Copy link
Member Author

mbg commented Mar 1, 2023

@michaelnebel thanks for reviewing! This should be all the work that's planned for this at the moment, so we can merge it if it's all looking good now. We can put it behind a feature flag, but I don't think we have done that for other languages which are already merged.

github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 1, 2023
View reviewed changes
@michaelnebel
Copy link
Contributor

michaelnebel commented Mar 2, 2023

@michaelnebel thanks for reviewing! This should be all the work that's planned for this at the moment, so we can merge it if it's all looking good now. We can put it behind a feature flag, but I don't think we have done that for other languages which are already merged.

No need to put it behind a feature flag then.
it was just in case you wanted to continue development without merging.

@michaelnebel michaelnebel self-requested a review March 2, 2023 11:46
michaelnebel
michaelnebel approved these changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

LGTM!

@mbg mbg merged commit fd9b279 into main Mar 2, 2023
@mbg mbg deleted the mbg/csharp/tsp-support branch March 2, 2023 11:47
@asgerf asgerf mentioned this pull request Mar 3, 2023
Merged
@mbg mbg restored the mbg/csharp/tsp-support branch March 3, 2023 12:34
@mbg mbg mentioned this pull request Mar 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelnebel michaelnebel michaelnebel approved these changes

@hvitved hvitved Awaiting requested review from hvitved

Assignees

@mbg mbg

Labels

C# C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mbg @michaelnebel @hvitved