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.

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

Change placement of DebuggerNonUserCodeAttribute #1735

Merged
merged 3 commits into from
Jul 7, 2016

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Jul 1, 2016

See #1671.

jskeet added 2 commits July 4, 2016 08:22
I think this has caught everything.
I've left a stub for attributes to be applied to the types themselves, but we don't currently need anything.

Follow-up commit will include the changes to generated code itself.

Fixes protocolbuffers#1671.
@jskeet jskeet force-pushed the attribute-placement branch from 8053c88 to c534845 Compare July 4, 2016 07:22
@jskeet jskeet assigned liujisi and unassigned jtattermusch Jul 5, 2016
@jskeet
Copy link
Contributor Author

jskeet commented Jul 5, 2016

Reassigned to @pherl is @jtattermusch is away. The Travis build failures are the ones afflicting the whole project, not just this PR.

@liujisi liujisi assigned anandolee and unassigned liujisi Jul 6, 2016
@liujisi
Copy link
Contributor

liujisi commented Jul 6, 2016

I added @anandolee to take a look.

printer->Print("[global::System.Diagnostics.DebuggerNonUserCodeAttribute]\n");
}

void SourceGeneratorBase::WriteGeneratedTypeAttributes(io::Printer* printer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a waste to have this empty method for future use only.
Let's add this when really needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My reasoning for keeping it was that we've already done the work to make sure it's called at all the right places. But I don't mind removing it - let's just make sure we keep that removal as a separate commit which is easy to revert. Will add that commit to this PR.

@anandolee
Copy link
Contributor

LGTM

@jskeet
Copy link
Contributor Author

jskeet commented Jul 7, 2016

Thanks. Will look into the CI failures, and submit if they're unrelated.

This does not affect the generated code.
If we decide we want to apply attributes to generated types, we should start by
just reverting this change.
@jskeet jskeet force-pushed the attribute-placement branch from 1cabcd7 to e9a7fc8 Compare July 7, 2016 19:20
@jskeet jskeet merged commit 03d9e09 into protocolbuffers:master Jul 7, 2016
@jskeet jskeet deleted the attribute-placement branch March 5, 2019 12:00
bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023
Change placement of DebuggerNonUserCodeAttribute
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.

5 participants