Skip to content

Conversation

@wilzbach
Copy link
Contributor

As discussed in dlang/phobos#5529 (comment)

FWIW at some point we might want to agree on an idiomatic ordering on the attributes
Alpha

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

dstyle.dd Outdated
matching attributes (`@nogc`, `@safe`, `pure`, `nothrow`).)
$(LI $(I Templated) functions should $(B not) be annotated with attributes
as the compiler can infer them.)
$(LI Attributes should be listed in alphabetical ordering, e.g. `const @nogc nothrow pure @safe`)
Copy link
Member

Choose a reason for hiding this comment

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

Please also add "(the ordering should ignore the leading @").

@wilzbach wilzbach force-pushed the attributes-order branch from e7d8f02 to 9ffa90f Compare July 4, 2017 23:18
@wilzbach
Copy link
Contributor Author

wilzbach commented Jul 4, 2017

Please also add "(the ordering should ignore the leading @").

Done:

image

@andralex
Copy link
Member

andralex commented Jul 4, 2017

Great! Once approved, you can merge this yourself. Do you have the appropriate rights?

@dlang-bot dlang-bot merged commit 984be9f into dlang:master Jul 4, 2017
@wilzbach wilzbach deleted the attributes-order branch July 4, 2017 23:28
@wilzbach
Copy link
Contributor Author

wilzbach commented Jul 4, 2017

Great! Once approved, you can merge this yourself.

I try very hard to not to merge by own PRs ;-)

Do you have the appropriate rights?

Yup.

@andralex
Copy link
Member

andralex commented Jul 6, 2017

I try very hard to not to merge by own PRs ;-)

Merging your own PR after it was approved with suggestions by a member with write access is entirely legit. In fact it's the only possible flow on phabricator.

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.

3 participants