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

Be more precise when expression syntax ${{ }} can be omitted in if statement #27497

Merged
merged 5 commits into from
Sep 5, 2023

Conversation

programmer04
Copy link
Contributor

@programmer04 programmer04 commented Aug 16, 2023

Why:

Unfortunately, you can't always omit ${{ }} in the if statement and it's not documented anywhere. Consider below examples

name: test-expression

on:
  push:
    branches:
      - main

jobs:
    test:
        runs-on: ubuntu-latest
        steps:
        - run: echo 'Hello, world!'
    statement-if:
        runs-on: ubuntu-latest
        if: github.event_name == 'push' && !startsWith(github.ref, 'refs/heads/')
        steps:
        - run: echo 'Summer!'

the above works as expected, but the below that only differs by order of statements in if doesn't work

name: test-expression

on:
  push:
    branches:
      - main

jobs:
    test:
        runs-on: ubuntu-latest
        steps:
        - run: echo 'Hello, world!'
    statement-if:
        runs-on: ubuntu-latest
        if: !startsWith(github.ref, 'refs/heads/') && github.event_name == 'push'
        steps:
        - run: echo 'Summer!'

ends with

image

This is due to the YAML format see

basically, it can't start with !, because it has meaning in YAML specification.

In my opinion, it's a pain to have it like that and it should be forbidden at all or work as expected even at the cost of breaking a little bit of specification of YAML. It should be reported to the team responsible for GitHub Workflows to take some action.

What's being changed (if available, include any code snippets, screenshots, or gifs):

The description for the if conditional.

Check off the following:

  • I have reviewed my changes in staging, available via the View deployment link in this PR's timeline.

    • For content changes, you will also see an automatically generated comment with links directly to pages you've modified. The comment won't appear if your PR only edits files in the data directory.
  • For content changes, I have completed the self-review checklist.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Aug 16, 2023
@programmer04 programmer04 changed the title Be more precise when expression syntax ${{ }} can be omitted Be more precise when expression syntax ${{ }} can be omitted in if statement Aug 16, 2023
@sarahmae1623

This comment was marked as spam.

@hoangnup868

This comment was marked as spam.

@cmwilson21
Copy link
Contributor

@programmer04 Thanks so much for submitting a PR! I'll get this triaged for review ⚡

@cmwilson21 cmwilson21 added content This issue or pull request belongs to the Docs Content team waiting for review Issue/PR is waiting for a writer's review actions This issue or pull request should be reviewed by the docs actions team and removed triage Do not begin working on this issue until triaged by the team labels Aug 18, 2023
Copy link
Contributor

@jc-clark jc-clark left a comment

Choose a reason for hiding this comment

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

Thank you @programmer04 for finding this and helping with the fix!

While I was searching around the other articles, I found some text on if conditionals in the "Variables" article as well.

What do you think about updating this line with the same text?

data/reusables/actions/expression-syntax-if.md Outdated Show resolved Hide resolved
@cmwilson21 cmwilson21 added the more-information-needed More information is needed to complete review label Aug 23, 2023
@programmer04
Copy link
Contributor Author

Thanks for the review and suggestion @jc-clark! For this syntax, it's even a little bit more complicated, because you can also write such expressions as if: !cancelled as e.g.:

  • if: '!cancelled'
  • if: (!cancelled)

read discussion and the other

@jc-clark
Copy link
Contributor

🤔 I see what you're saying @programmer04. I think it would make sense to document the two examples you listed.

I think the paragraph in the data/reusables/actions/expression-syntax-if.md reusable is starting to get a bit long and difficult to understand. Ideally we can split this paragraph into a couple smaller paragraphs.

There's one problem though. We can't just add line breaks because the reusable also exists in a bulleted list on this line. Adding line breaks will break the formatting in the bulleted list.

It seems to me that we can delete this bullet point in the note since the same information is at the top of the article. Then we'd be able to add some line breaks and more information to the reusable.

What do you think of that?

@cmwilson21
Copy link
Contributor

👋 @programmer04 - just checking in. Is this one ready for another review?

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.


Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
actions/learn-github-actions/expressions.md fpt
ghec
ghes@ 3.10 3.9 3.8 3.7 3.6
ghae
fpt
ghec
ghes@ 3.10 3.9 3.8 3.7 3.6
ghae

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server
ghae: GitHub AE

@programmer04
Copy link
Contributor Author

I made suggested adjustments, it's ready for review @jc-clark and @cmwilson21. Btw. it would be easier if GH would always require ${{ }} in if statements 😅

@programmer04 programmer04 force-pushed the improve-if-docs branch 2 times, most recently from ac68dae to 148445e Compare September 1, 2023 15:46
jc-clark and others added 2 commits September 5, 2023 12:02
Co-authored-by: Joe Clark <31087804+jc-clark@users.noreply.github.com>
@jc-clark
Copy link
Contributor

jc-clark commented Sep 5, 2023

Looks great to me. Many thanks for helping keep this updated @programmer04. I'll get this set up to merge.

@jc-clark jc-clark added this pull request to the merge queue Sep 5, 2023
Merged via the queue into github:main with commit 8ee4d38 Sep 5, 2023
32 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues

@programmer04 programmer04 deleted the improve-if-docs branch September 6, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team more-information-needed More information is needed to complete review waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants