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

LineLengths=110 #104

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

LineLengths=110 #104

wants to merge 2 commits into from

Conversation

mattBrzezinski
Copy link
Member

Resolves: #78

@@ -36,7 +36,11 @@ When convention guidelines conflict this guide takes precedence (known conflicts
- Avoid padding brackets with spaces. ex. `Int64(value)` preferred over `Int64( value )`.

## Contents
- [Code Formatting](#code-formatting)
- [Blue: a Style Guide for Julia](#blue-a-style-guide-for-julia)
- [A Word on Consistency](#a-word-on-consistency)
Copy link
Member Author

Choose a reason for hiding this comment

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

VSCode just autosaves this back to be indented? 🤷

@@ -26,7 +26,7 @@ Attempt to follow both the [Julia Contribution Guidelines](https://github.com/Ju
When convention guidelines conflict this guide takes precedence (known conflicts will be noted in this guide).

- Use 4 spaces per indentation level, no tabs.
- Try to adhere to a 92 character line length limit.
- Try to adhere to a 110 character line length limit.
Copy link

Choose a reason for hiding this comment

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

Can you add a blurb or comment about why it's 110 characters? It would make it easier to update in the future

(or just a blurb in the MR)

@nickrobinson251
Copy link
Contributor

nickrobinson251 commented May 18, 2022

I think this is premature. Blue Style is used by hundreds of people and packages outside Invenia, and this is a highly disruptive change (at least when rolled out via the auto formatter). I'd think of this along the lines of a breaking release of a very widely used package that's been stable for a long time.

On a similar, but slightly different poin to the disruptiveness, I also think changing the opinion of the style guide somewhat goes against the spirit of a style guide, that is, the style guide gives the answer to disputed style questions. Where there are currently no answer, we can decide to take an opinion, but changing an answer is a different matter. I kinda think if line length is disputed, we should do what the style guide already says. (And i'm assuming it is disputed i.e. "increase the line length" wouldn't have near universal agreement. If it does, then let's consider it and pick a number (110 could be good, idk), but we have to have figure out the disruptiveness concern).

So I'm not saying this couldn't change, just that it should be well motivated/justified and probably have wider discussion and agreement, imo.

@mattBrzezinski
Copy link
Member Author

I think this is premature. Blue Style is used by hundreds of people and packages outside Invenia, and this is a highly disruptive change (at least when rolled out via the auto formatter). I'd think of this along the lines of a breaking release of a very widely used package that's been stable for a long time.

On a similar, but slightly different poin to the disruptiveness, I also think changing the opinion of the style guide somewhat goes against the spirit of a style guide, that is, the style guide gives the answer to disputed style questions. Where there are currently no answer, we can decide to take an opinion, but changing an answer is a different matter. I kinda think if line length is disputed, we should do what the style guide already says. (And i'm assuming it is disputed i.e. "increase the line length" wouldn't have near universal agreement. If it does, then let's consider it and pick a number (110 could be good, idk), but we have to have figure out the disruptiveness concern).

So I'm not saying this couldn't change, just that it should be well motivated/justified and probably have wider discussion and agreement, imo.

This issue has been open for 1.5years. I think a better solution might just be me setting up formatters so I can style the code however I want locally, then format according to an agreed upon standard before committing changes. That way all these pointless discussions never have to happen. 🤔

@oxinabox
Copy link
Member

One thing is this is not a "breaking" change.
Decreasing the line limit is breaking, increasing it is fine.
Anything that followed the guide before still follows it afterwards.
Though we often make breaking changes when we do add a rule for something the guide had no opinion on.

I do agree this is likely to be very disruptive

@nickrobinson251 nickrobinson251 removed their request for review May 19, 2022 11:31
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.

Line limit to 120
5 participants