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

Style guide (see #3) #42

Merged
merged 14 commits into from
Dec 31, 2019
Merged

Conversation

zbeekman
Copy link
Member

No description provided.

@zbeekman zbeekman mentioned this pull request Dec 22, 2019
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
@certik
Copy link
Member

certik commented Dec 24, 2019

Thanks for writing it down. I think this mostly captures the style guide that most people agree on. I left some comments.

STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
STYLE_GUIDE.md Outdated Show resolved Hide resolved
* Most editors respect `.editorconfig` settings.
* For those that don't out of the box plugins are readily available for all major editors at
  https://editorconfig.org/#download
Checkout/commit native line endings, even if users don't have `core.autocrlf` set.
STYLE_GUIDE.md: Shorten line length and use more semantic new-line breaks
Simplify the white-space section of STYLE_GUIDE.md and incorporate comments from
Milan and Certik.
* Recommend using names on end statements
* Allow exceptions for very short constructs that can be reasonably assumed to be on one screen, within ~25 lines
The first paragraph with quote from Mike McQuaid was replaced with a brief paragraph.
The new text aims to indicate why a style convention guide is useful and that it can be amended.
@zbeekman zbeekman marked this pull request as ready for review December 29, 2019 21:57
@zbeekman zbeekman changed the title Style guide enforcement (see #3) Style guide (see #3) Dec 29, 2019
@zbeekman
Copy link
Member Author

I've decided to not add the enforcement half of this work quite yet, and let this PR be a place to discuss what the style guide is, before any effort to enforce it is made. (After all, setting style conventions seems to be a necessary prerequisite to trying to enforce any.)

Add @longb's suggestion to only use standardized language features
and not obsolescent or deleted features and vendor extensions.
Thanks to @ivan-pi for pointing this out.
Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Thanks for this work. I think it's good to merge. We could spend a long time working out the details (and we will) but I think it's important to have a rough draft in master early. Most ideas for improving this we'll get as we review future PRs as they come in. We can continue this work with more focused PRs on specific items in the style guide.

@zbeekman
Copy link
Member Author

@milancurcic Just out of curiosity, do we want to codify/formalize how code reviews and PRs work? When is it OK to merge? By whom? This gets into a bigger issue: governance, but we can punt on that for a bit.

@milancurcic
Copy link
Member

Yes, we should discuss that in #5 and come up with some workflow to start with.

Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

Otherwise it's fine with me. I don't agree with the two spaces of formatting and some of the details of the naming conventions, but we have bigger fish to fry. After the spelling fixes, let's merge this.

STYLE_GUIDE.md Outdated Show resolved Hide resolved
Co-Authored-By: Ondřej Čertík <[email protected]>
@zbeekman zbeekman requested a review from certik December 31, 2019 02:00
@certik
Copy link
Member

certik commented Dec 31, 2019 via email

@milancurcic milancurcic merged commit 173d8fe into fortran-lang:master Dec 31, 2019
@zbeekman zbeekman deleted the style-enforcement branch January 29, 2020 02:44
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