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

Remove 'Defined as:' #4134

Closed
coke opened this issue Nov 20, 2022 · 7 comments
Closed

Remove 'Defined as:' #4134

coke opened this issue Nov 20, 2022 · 7 comments
Assignees
Labels
meta RFCs, general discussion, writing style, repository organization, etc.

Comments

@coke
Copy link
Collaborator

coke commented Nov 20, 2022

Looks like there was a style at some point to use

Defined as:

at the top of a routine before the method signature. I think this is unnecessary and we can just lead with the method declaration.

@coke coke added the meta RFCs, general discussion, writing style, repository organization, etc. label Nov 20, 2022
@coke
Copy link
Collaborator Author

coke commented Nov 20, 2022

Another ticket mentioned Declared as, which had only one use and was recently removed.

@2colours
Copy link
Contributor

@coke could you show the ticket with "Declared as"?

I don't feel strongly about this one, either "for" or "against". Perhaps it could go but again, it would be good to get opinions from others who might be interested. @0rir what do you think, for example?

@0rir
Copy link
Contributor

0rir commented Nov 24, 2022

I didn't find the issue mentioning it, so my opinion is tentative.

As documentation, I think it can go as non-informative. And it is optional in xt/check-signatures.t 's grammar TypeDocumentation. That suggests it is just a waste of space.

@coke
Copy link
Collaborator Author

coke commented Nov 24, 2022

#3441

@2colours
Copy link
Contributor

I'm trying to somewhat assure it doesn't hit anyone as a big surprise but otherwise I think the situation is in favor of the removal. It could even be removed from the tests and then we could quickly see where there are left-behinds.

@0rir
Copy link
Contributor

0rir commented Nov 25, 2022

Thanks, @coke, I guess I messed up with the quoting to find #3441.

There are roughly a thousand instances all, or nearly all, under /types. Reading through #3441, my opinion is affirmatively that those should go.

I won't volunteer because I have not dealt with any Not-Invented-By-Me grammars and feel my match object to action class tooling is weak. And, I feel like I should continue work on Issue #4117, even without feedback on my plan. (@Altai-man, is silence consent or lack of time.)

@Altai-man
Copy link
Member

I won't volunteer because I have not dealt with any Not-Invented-By-Me grammars and feel my match object to action class tooling is weak. And, I feel like I should continue work on Issue #4117, even without feedback on my plan. (@Altai-man, is silence consent or lack of time.)

It is lack of time mostly, due to dayjob. If you can suggest a PR for improvements to categories noted - we should have it, it will be a help.

@coke coke self-assigned this Dec 11, 2022
@coke coke closed this as completed in 0779d68 Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta RFCs, general discussion, writing style, repository organization, etc.
Projects
None yet
Development

No branches or pull requests

4 participants