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.

Sign up for GitHub

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

Jump to bottom

Create STYLE_GUIDE.md #1876

Merged
merged 1 commit into from
Nov 19, 2021
Merged

Create STYLE_GUIDE.md #1876

merged 1 commit into from
Nov 19, 2021

Conversation

FredKSchott
Copy link
Member

@FredKSchott FredKSchott commented Nov 18, 2021 •
edited
Loading

  • Adds a style guide!
  • Important: This adds zero new style rules to our project. It simply exists to outline our current style guide (as enforced by linters and formatters) and create a place to record style guide items that cannot be captured by linters in the future.
  • Additionally, this document sets out to define a process that style guide changes can be proposed and evaluated. The exact details are still TBD, but I believe that we have the ability to flesh this out in a trial run (non-binding) before finalizing into our GOVERNANCE doc.

This was created in response to the discussion in #1695 and other PRs where style changes had been proposed but fell out of scope for the change at hand.

@FredKSchott FredKSchott requested a review from a team as a code owner November 18, 2021 07:05
@changeset-bot
Copy link

changeset-bot bot commented Nov 18, 2021

⚠️ No Changeset found

Latest commit: e5ce98f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented Nov 18, 2021 •
edited
Loading

✔️ Deploy Preview for astro-www ready!

🔨 Explore the source changes: e5ce98f

🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-www/deploys/6195fb23ad86990007a9f103

😎 Browse the preview: https://deploy-preview-1876--astro-www.netlify.app

@netlify
Copy link

netlify bot commented Nov 18, 2021 •
edited
Loading

✔️ Deploy Preview for astro-docs-2 ready!

🔨 Explore the source changes: e5ce98f

🔍 Inspect the deploy log: https://app.netlify.com/sites/astro-docs-2/deploys/6195fb23cbb84f0007479c31

😎 Browse the preview: https://deploy-preview-1876--astro-docs-2.netlify.app

drwpow
drwpow reviewed Nov 18, 2021
View reviewed changes
STYLE_GUIDE.md

Anything else in this document -- that is, anything not automatically enforced by linting -- is considered **style guidance.** Do your best to follow all style guidance outlined in this style guide, and expect code reviewers to be looking for these things in your Pull Request. However, no tool exists to catch you when you break guidance.

The reason that we don't treat these as strict rules is simple: we are all human. It would be silly to expect 100% accuracy without automated tooling to enforce it. Authors and reviewers will both miss things every once-in-a-while, and that is okay.
Copy link
Member

Choose a reason for hiding this comment

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

❤️

drwpow
drwpow reviewed Nov 18, 2021
View reviewed changes
STYLE_GUIDE.md

Sometimes, not everyone will agree on style changes and 100% consensus is impossible. This is a condition commonly referred to as bike-shedding. If consensus can not be reached, a simple majority vote among core contributors (L3) will suffice.

_Note: This process is new, we are still figuring it out! This process will be moved into GOVERNANCE.md when finalized._
Copy link
Member

Choose a reason for hiding this comment

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

Love this section! Well-said

drwpow
drwpow approved these changes Nov 18, 2021
View reviewed changes
Copy link
Member

@drwpow drwpow left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for pushing this forward.

@FredKSchott FredKSchott merged commit ebe138b into main Nov 19, 2021
@FredKSchott FredKSchott deleted the FredKSchott-patch-1 branch November 19, 2021 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewp matthewp matthewp approved these changes

@drwpow drwpow drwpow approved these changes

@natemoo-re natemoo-re natemoo-re approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@FredKSchott @matthewp @drwpow @natemoo-re