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

MINOR: Fix docstring style #445

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

MINOR: Fix docstring style #445

wants to merge 1 commit into from

Conversation

alkis
Copy link
Contributor

@alkis alkis commented Jul 8, 2024

Rationale for this change

Make docstring style consistent.

What changes are included in this PR?

  1. all comments are 80 cols wide
  2. inline comments: // inline-comment
  3. single line: /** single line */
  4. multi line:
/**
 * multi
 * line
 */
  1. when a field is deprecated we use DEPRECATED: ...
  2. move deprecated structs to the end of the file

Do these changes have PoC implementations?

n/a.

1. all comments are 80 cols wide
2. inline comments: `// inline-comment`
3. single line: `/** single line */`
4. multi line:
```
/**
 * multi
 * line
 */
5. when a field is deprecated we use `DEPRECATED: ...`
6. move deprecated structs to the end of the file
@alkis
Copy link
Contributor Author

alkis commented Jul 9, 2024

FYI if it was up to me I would move away from java style and go to all // comments. This means less rules to follow and they would be more compact to boot. If reviewers are happy with this I can make the change.

@wgtmac
Copy link
Member

wgtmac commented Jul 9, 2024

I wonder if there is any good linter for thrift

@Fokko
Copy link
Contributor

Fokko commented Jul 9, 2024

I would be reluctant to merge this because it rewrites a lot of history. Of course, you can still get to it by referencing the previous commit, but for a specification like this I think it is extra important to keep the history as clean as possible.

@alkis
Copy link
Contributor Author

alkis commented Jul 11, 2024

I would be reluctant to merge this because it rewrites a lot of history. Of course, you can still get to it by referencing the previous commit, but for a specification like this I think it is extra important to keep the history as clean as possible.

I can relate to the sentiment but I feel it applies more strongly to code. Documentation is less critical to have super clean history on.

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.

3 participants