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

Share SourceWriter between JSON & config binding generators #89150

Merged
merged 2 commits into from
Jul 19, 2023
Merged

Share SourceWriter between JSON & config binding generators #89150

merged 2 commits into from
Jul 19, 2023

Conversation

layomia
Copy link
Contributor

@layomia layomia commented Jul 19, 2023

Fixes #88918. FYI @tannergooding @ericstj @jasper-d @martincostello @vcsjones @lateralusX.

The writer used for the JSON generator has the correct right line-ending behavior.

@layomia layomia added this to the 8.0.0 milestone Jul 19, 2023
@layomia layomia self-assigned this Jul 19, 2023
@ghost
Copy link

ghost commented Jul 19, 2023

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #88918. FYI @tannergooding @ericstj @jasper-d @martincostello @vcsjones @lateralusX.

The writer used for the JSON generator has the correct right line-ending behavior.

Author: layomia
Assignees: layomia
Labels:

area-Extensions-Configuration
Milestone: 8.0.0

@layomia layomia force-pushed the binder-gen-sourcewriter branch 3 times, most recently from b3162e0 to 65c718f Compare July 19, 2023 02:10
@build-analysis build-analysis bot mentioned this pull request Jul 19, 2023
Closed
@layomia
Copy link
Contributor Author

layomia commented Jul 19, 2023

/backport to release/8.0-preview7

@github-actions
Copy link
Contributor

Started backporting to release/8.0-preview7: https://github.com/dotnet/runtime/actions/runs/5602367197

@github-actions github-actions bot mentioned this pull request Jul 19, 2023
Merged
@eiriktsarpalis
Copy link
Member

Shouldn't this be merged before initiating a backport PR?

@ericstj
Copy link
Member

ericstj commented Jul 19, 2023 •
edited
Loading

Shouldn't this be merged before initiating a backport PR?

We're close on time -- it's fine to merge in parallel so long as we squash the commits in release. It does mean that both PRs need to be kept in sync, but right now we're ready to merge and just waiting for the validation build to complete. Assuming both are green we save a couple hours by doing it parallel.

@layomia
Copy link
Contributor Author

layomia commented Jul 19, 2023

Build failure appears unrelated.

@layomia layomia merged commit 7036cba into dotnet:main Jul 19, 2023
@build-analysis build-analysis bot mentioned this pull request Jul 19, 2023
Closed
@layomia layomia mentioned this pull request Jul 21, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ericstj ericstj ericstj left review comments

@eiriktsarpalis eiriktsarpalis eiriktsarpalis approved these changes

@eerhardt eerhardt Awaiting requested review from eerhardt

@tarekgh tarekgh Awaiting requested review from tarekgh

Assignees

@layomia layomia

Labels
area-Extensions-Configuration
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

.\build.cmd -subset clr+libs -config release fails in ConfigurationBindingGenerator
3 participants
@layomia @eiriktsarpalis @ericstj