Skip to content

Reserve feature branch error codes#45567

Merged
RikkiGibson merged 3 commits intodotnet:release/dev16.7from
RikkiGibson:fix-error-codes
Jul 1, 2020
Merged

Reserve feature branch error codes#45567
RikkiGibson merged 3 commits intodotnet:release/dev16.7from
RikkiGibson:fix-error-codes

Conversation

@RikkiGibson
Copy link
Member

#44911 ended up using error codes which are in use in a feature branch. Since it's tricky to pack these error codes together I'd like to just move the LangVersion 9 error code over to a single unoccupied space in the C# 9 range, if that's acceptable. Also using a more obvious method to indicate that the error codes are in use in feature branches. Hopefully this will all be moot in a few weeks after we've finished feature review and ensured test coverage, etc. on par with our standards.

@RikkiGibson RikkiGibson requested a review from chsienki June 30, 2020 20:32
@RikkiGibson RikkiGibson requested a review from a team as a code owner June 30, 2020 20:32
ERR_8815 = 8815,

[Obsolete("used by features/module-initializers")]
ERR_8816 = 8816,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need [Obsolete] attributes? Would the comment plus placeholders be sufficient?

// Codes 8813, ... used by features/module-initializers
ERR_8813 = 8813,
...

Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

@RikkiGibson, won't this cause merge pain for these branches? Every time they make a change in this file, merging master in could be just as annoying as it is today.

@RikkiGibson
Copy link
Member Author

The pain without this "reserving" approach is having to find new error codes if yours get taken, the pain after this approach is basically taking the feature branch version of the affected lines and fixing compile errors by deleting references in DiagnosticTest.cs.

To be honest I normally wouldn't bother with a change like this, it's just that 1. I'd like to keep the error codes for a feature together, 2. the codes for these branches pack really well in these spots, and 3. the feature branches will merge fairly soon.

@chsienki
Copy link
Member

chsienki commented Jul 1, 2020

LGTM. I wonder if we can come up with some mechanism of reserving across branches in the future. Something as simple as a shared excel sheet on a sharepoint that lets us track the various ones across branches. This seems to be a common pain point in feature branch merges...

@RikkiGibson RikkiGibson merged commit 6ff210c into dotnet:release/dev16.7 Jul 1, 2020
@RikkiGibson RikkiGibson deleted the fix-error-codes branch July 1, 2020 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants