Skip to content

Fix indentation inside Code Examples #271

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

Merged
merged 4 commits into from
Oct 15, 2020
Merged

Fix indentation inside Code Examples #271

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0ef059
Fix indentation inside Code Examples
dtinth Oct 8, 2020
b52750e
Remove leading indentation in Code Examples
dtinth Oct 15, 2020
c17af6f
Merge branch 'main' into patch-1
rachmari Oct 15, 2020
642f4b3
Merge branch 'main' into patch-1
rachmari Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions contributing/content-style-guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,15 @@ When code examples refer to a larger file, show the relevant section of the file

```
on:
schedule:
- cron: "40 19 * * *"
schedule:
- cron: "40 19 * * *"
```

- **Avoid:**

```
schedule:
- cron: "40 19 * * *"
schedule:
Copy link
Contributor

@rachmari rachmari Oct 10, 2020

Choose a reason for hiding this comment

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

Because we always start code blocks without any indentation, what do you think about reverting the indents on lines 128 and 129?

So the code example would be:

Use

on:
  schedule:
    - cron:  "40 19 * * *"

Avoid

schedule:
  - cron:  "40 19 * * *"

Copy link
Contributor Author

@dtinth dtinth Oct 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@rachmari I think we just ran into a paradox 😂… As the code block in question is about “what to avoid” (which in my opinion should be exempt from the guideline).

  • The rule to “start code blocks without any indentation” also reinforces the guideline being addressed (to include the necessary context in code samples). Without the indentation the code example wouldn’t make sense; see the next point…

  • In workflow files, schedule is not a top-level property; it must be nested inside the on: block. YAML is whitespace-sensitive so removing the indentation will make the workflow file invalid. To show a valid part of a valid workflow file (which is what I am trying to convey) the indentation must be kept intact.

    ⬇️ This is not part of a valid workflow file (due to the missing indentation), so it might not be a good example...

    Avoid

    schedule:
  - cron:  "40 19 * * *"

    ⬇️ This is part of a valid workflow file but not following the guideline because the context is not included...

    Avoid

      schedule:
    - cron:  "40 19 * * *"

Let me know what you think. If you disagree with this I will make the changes as requested. Thanks!

Copy link
Contributor

@rachmari rachmari Oct 14, 2020

Choose a reason for hiding this comment

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

@dtinth, I see what you mean here about the paradox. 😂

The purpose of the style guideline is to let contributors know that when using code examples logical boundaries of the code example should be included. For example, if it were a javascript code sample it might be most relevant to include a whole function rather than a snippet inside a function. This specific example is referring to a GitHub Actions workflow file and specifically highlighting that you should include the top-level property on that schedule is related` to.

Sometimes we don't use the whole GitHub Actions workflow but just refer to steps. In that case, we ensure that we include the steps property at the top of the file.

for example:

steps:
 - name: My first step
   if: ${{ github.event_name == 'pull_request' && github.event.action == 'unassigned' }}
   run: echo This event is a pull request that had an assignee removed.

I suggested not indenting the Avoid example because it would conflate two style guidelines in one example. This specific example is referring to the guideline to show logic boundaries in code samples, not to avoid indenting the first line in a code sample. I think that would be a new style guideline.

Copy link
Contributor Author

@dtinth dtinth Oct 15, 2020

Choose a reason for hiding this comment

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

@rachmari Thanks for taking time to explanation the rationale. 🙏

Having seen other parts of the docs, this now becomes clear that indentation in an action’s subsection is omitted. I have fixed the Avoid section in the code example as you suggested. 😃

Copy link
Contributor

@rachmari rachmari Oct 15, 2020

Choose a reason for hiding this comment

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

@dtinth

Having seen other parts of the docs, this now becomes clear that indentation in an action’s subsection is omitted.

I think the underlying issue is that we're not specific enough about what this guideline means in our style guide. We should improve that in the future.

- cron: "40 19 * * *"
```

## Headers
Expand Down