Skip to content

Refactor: fix mismatch of Scope class member declaration#5751

Merged
WalterBright merged 2 commits intodlang:masterfrom
9rnsr:sync_cpp_header
Jul 2, 2016
Merged

Refactor: fix mismatch of Scope class member declaration#5751
WalterBright merged 2 commits intodlang:masterfrom
9rnsr:sync_cpp_header

Conversation

@9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented May 9, 2016

Separated from #5750.

@mathias-lang-sociomantic
Copy link
Contributor

Why the change of style ? Otherwise 👍

@9rnsr
Copy link
Contributor Author

9rnsr commented May 9, 2016

Without the style change, the bugfix would break vertical alignment of line comments in scope.h. I think placing comment in own line would be better to reduce bike-shed discussion.

@mathias-lang-sociomantic
Copy link
Contributor

Without the style change, the bugfix would break vertical alignment of line comments in scope.h. I think placing comment in own line would be better to reduce bike-shed discussion.

Fine by me (I tend to prefer this style anyway).
Maybe we could have a style guide (as part of a CONTRIBUTING.md file) to reduce the amount of bike shedding even more ?

@9rnsr
Copy link
Contributor Author

9rnsr commented May 9, 2016

Yes... I think we should do it as fast as possible.

Walter is not active for such the guideline documents, then someone should work for that.

To me writing long documentation is much painful than contributing code, because I'm not a native English user. But I would have to do it...

@mathias-lang-sociomantic
Copy link
Contributor

I'll gladly help if you can provide the key point.
Could you contact me via email ? Both my personal and company email are on bugzilla under my real name.

@9rnsr
Copy link
Contributor Author

9rnsr commented May 9, 2016

Thanks for your offer, but the guideline would be a list of my points in the first time. If it would not fit the Walter's points while reviewing, your work would be wasted. So first I should write it in my responsibility.
After the PR opened, I'll welcome review comments.

src/dscope.d Outdated
DeprecatedDeclaration depdecl; // customized deprecation message

// customized deprecation message
DeprecatedDeclaration depdecl;
Copy link
Member

Choose a reason for hiding this comment

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

I see no reason for this change. It's just churn.

@yebblies
Copy link
Contributor

yebblies commented May 9, 2016

I think placing comment in own line would be better to reduce bike-shed discussion.

It seems you're wrong about this.

@9rnsr 9rnsr force-pushed the sync_cpp_header branch from 404984c to bdd1747 Compare May 16, 2016 02:00
@WalterBright
Copy link
Member

Auto-merge toggled on

@WalterBright WalterBright merged commit 15220d9 into dlang:master Jul 2, 2016
@9rnsr 9rnsr deleted the sync_cpp_header branch July 10, 2016 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants