Skip to content

Consolidate csproj writing#4027

Merged
ArcturusZhang merged 27 commits intoAzure:feature/v3from
ArcturusZhang:consolidate-csproj-writing
Dec 24, 2023
Merged

Consolidate csproj writing#4027
ArcturusZhang merged 27 commits intoAzure:feature/v3from
ArcturusZhang:consolidate-csproj-writing

Conversation

@ArcturusZhang
Copy link
Copy Markdown
Member

@ArcturusZhang ArcturusZhang commented Dec 9, 2023

Description

Reason of making this change

This change uncovers an issue of the sequence of reading and writing csproj.
The autorest pipeline might parallelize the plugins when they do not have dependencies to each other, in our case, it is csharpgen and csharpproj, they are running in parallel - and in this change, we read csproj in csharpgen, and write csproj in csharpproj which might lead to read file exception because the file is locked to be edited.

We once introduced a partial solution in this issue when we first find the issue with the same root cause, by introducing a CSProjConfiguration class to hold those configurations that are needed when writing csproj file.

Since these configuration should never have a different value from the one when writing source code, and we will never have a concurrent issue when we put them into the same plugin, this class (and its corresponding test cases) is not need therefore removed in this PR.

Changes included in this PR

  1. remove the csharpproj plugin (and all the other types and test cases related to it), and move the functionality of this plugin (writes a csproj file) to csharpgen plugin.
  2. consolidate the way of writing csproj and other xml document to use XmlWriter instead to make it to have semantics.
  3. (WIP) consolidate the writer of writing csproj files into the same class (NewProjectScaffolding class) because we previously have multiple classes doing the same thing.

Follow ups needed to be done after this PR: #4037

Checklist

To ensure a quick review and merge, please ensure:

  • The PR has a understandable title and description explaining the why and what.
  • The PR is opened in draft if not ready for review yet.
    • If opened in draft, please allocate sufficient time (24 hours) after moving out of draft for review
  • The branch is recent enough to not have merge conflicts upon creation.

Ready to Land?

  • Build is completely green
    • Submissions with test failures require tracking issue and approval of a CODEOWNER
  • At least one +1 review by a CODEOWNER
  • All -1 reviews are confirmed resolved by the reviewer
    • Override/Marking reviews stale must be discussed with CODEOWNERS first

@ArcturusZhang

This comment was marked as outdated.

@ArcturusZhang ArcturusZhang marked this pull request as ready for review December 12, 2023 02:22
@ArcturusZhang ArcturusZhang added Mgmt This issue is related to a management-plane library. DPG labels Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DPG Mgmt This issue is related to a management-plane library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants