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.

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

Mark compiler generated files as generated for GitHub PRs #31564

Merged
merged 1 commit into from Dec 6, 2018

Conversation

Projects
None yet
4 participants
@agocke
Copy link
Contributor

agocke commented Dec 5, 2018

GitHub will recognize these attributes and collapse generated files by-default when you're doing a pull request. They're not hidden completely -- you can still expand the file if you want to see the generated code.

@agocke agocke requested review from jaredpar and dotnet/roslyn-compiler Dec 5, 2018

@jcouv jcouv self-assigned this Dec 5, 2018

@jcouv

jcouv approved these changes Dec 5, 2018

Copy link
Member

jcouv left a comment

LGTM Thanks

@jcouv

This comment has been minimized.

Copy link
Member

jcouv commented Dec 5, 2018

I wonder if we should include the XLF files in that list as well.

@agocke

This comment has been minimized.

Copy link
Contributor

agocke commented Dec 5, 2018

Talking about this offline, apparently localization people read those when they're doing PRs, so it would create a lot of work for loc.


# Generated files
src/Compilers/CSharp/Portable/Generated/* linguist-generated=true
src/Compilers/CSharp/Portable/CSharpResources.Designer.cs linguist-generated=true

This comment has been minimized.

@jasonmalinowski

jasonmalinowski Dec 6, 2018

Member

Why just this and not all .Designer.cs? Or should this line be pushed to linguist's actual implementation?

This comment has been minimized.

@agocke

agocke Dec 6, 2018

Contributor

I wasn't sure everything *.Designer.cs was generated.

@jaredpar

This comment has been minimized.

Copy link
Member

jaredpar commented Dec 6, 2018

Talking about this offline, apparently localization people read those when they're doing PRs, so it would create a lot of work for loc

The GitHub documentation says this setting only affects the default diff display. If that's the case then LOC reviews owuld just need to unhide the files before reviewing. If this is not the case, and the setting completely hides files from view, then I would reject this change. I don't like reviewing generated files all the time but I want the capability to do so if needed.

@agocke

This comment has been minimized.

Copy link
Contributor

agocke commented Dec 6, 2018

@jaredpar It doesn't hide them completely, just collapses the view.

@jaredpar

This comment has been minimized.

Copy link
Member

jaredpar commented Dec 6, 2018

It doesn't hide them completely, just collapses the view.

Given the review to no review nature of XLIFF files I think we should consider it.

@agocke

This comment has been minimized.

Copy link
Contributor

agocke commented Dec 6, 2018

@jaredpar I think you need to negotiate with @jasonmalinowski for that.

@agocke agocke merged commit ac72433 into dotnet:master Dec 6, 2018

3 checks passed

license/cla All CLA requirements met.
roslyn-CI #20181205.30 succeeded
Details
roslyn-integration-CI #20181205.28 succeeded
Details

@agocke agocke deleted the agocke:new-nullable-diagnostic-design branch Dec 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment