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.

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

High Level overview #10319

Merged
merged 31 commits into from
Jul 26, 2024
Merged

High Level overview #10319

merged 31 commits into from
Jul 26, 2024

Conversation

maridematte
Copy link
Contributor

Fixes #10215

A document on the high level MSBuild parts and execution.

@maridematte
Copy link
Contributor Author

There are a few topics that are not covered here or deserve a bit more about them. These are not a full list:

  • MSBuild server
  • Built-in tasks and targets: some more widely used examples.
  • PerfStar usage

f-alizada
f-alizada reviewed Jul 2, 2024
View reviewed changes
Copy link
Contributor

@f-alizada f-alizada left a comment

Choose a reason for hiding this comment

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

Thank you @maridematte for the document, it is very useful!
I left some comments to understand/clarify things

documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
JanKrivanek
JanKrivanek reviewed Jul 2, 2024
View reviewed changes
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Overall looks good.
It's a lot of information and text - so some design chart(s) might help better clarity

documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
Nirmal4G
Nirmal4G suggested changes Jul 9, 2024
View reviewed changes
Copy link
Contributor

@Nirmal4G Nirmal4G left a comment

Choose a reason for hiding this comment

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

You also have <Sdk Name="My.Build.Sdk" Version="1.0.0" /> element.

rainersigwald
rainersigwald reviewed Jul 15, 2024
View reviewed changes
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
JanKrivanek
JanKrivanek reviewed Jul 22, 2024
View reviewed changes
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Overall this is very informative and helpful!
It looks very close to done for v1. The only thing holding me from signoff is the couple of the TODO sections (let's address or remove those).

documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
@JanKrivanek JanKrivanek self-requested a review July 24, 2024 17:44
JanKrivanek
JanKrivanek approved these changes Jul 24, 2024
View reviewed changes
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Amazing!!
Feels ready to go as V1

documentation/High-level-overview.md Outdated Show resolved Hide resolved
f-alizada
f-alizada approved these changes Jul 25, 2024
View reviewed changes
Copy link
Contributor

@f-alizada f-alizada left a comment

Choose a reason for hiding this comment

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

Thank you!

documentation/High-level-overview.md Outdated Show resolved Hide resolved
@MichalPavlik
Copy link
Member

As the document mentions the FileTracker, should we also mention the Locator?

MichalPavlik
MichalPavlik reviewed Jul 25, 2024
View reviewed changes
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
documentation/High-level-overview.md Outdated Show resolved Hide resolved
AR-May
AR-May approved these changes Jul 25, 2024
View reviewed changes
Copy link
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

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

LGTM

documentation/High-level-overview.md Outdated Show resolved Hide resolved
@maridematte maridematte enabled auto-merge (squash) July 26, 2024 09:03
@maridematte maridematte merged commit 59c2ff8 into dotnet:main Jul 26, 2024
10 checks passed
rainersigwald
rainersigwald reviewed Jul 26, 2024
View reviewed changes
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

I am so sorry, I apparently didn't POST my feedback.

documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
documentation/High-level-overview.md Show resolved Hide resolved
@maridematte maridematte mentioned this pull request Jul 30, 2024
Merged
maridematte added a commit that referenced this pull request Aug 5, 2024
@maridematte
Overview doc extra review (#10461)
85d805a 
Addressing some comments from this PR: #10319 that were made after it was merged.
@maridematte maridematte deleted the docOverview branch August 7, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f-alizada f-alizada f-alizada approved these changes

@JanProvaznik JanProvaznik JanProvaznik left review comments

@Nirmal4G Nirmal4G Nirmal4G requested changes

@rainersigwald rainersigwald rainersigwald left review comments

@AR-May AR-May AR-May approved these changes

@MichalPavlik MichalPavlik MichalPavlik approved these changes

@JanKrivanek JanKrivanek JanKrivanek approved these changes

@baronfel baronfel Awaiting requested review from baronfel

@YuliiaKovalova YuliiaKovalova Awaiting requested review from YuliiaKovalova

@surayya-MS surayya-MS Awaiting requested review from surayya-MS

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MSBuild High Level Overview document
8 participants
@maridematte @MichalPavlik @rainersigwald @JanKrivanek @Nirmal4G @JanProvaznik @AR-May @f-alizada