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

Improved the example of Setting an Environment Var #2488

Merged
merged 7 commits into from
Jan 29, 2021

Conversation

espipj
Copy link
Contributor

@espipj espipj commented Jan 2, 2021

Why:

Current example is not self explanatory. I needed to browse through GitHub issues till I found actions/starter-workflows#68 (comment) which worked flawlessly.

I think this change will help loads of users of GH Actions as this (not being able to use ::set-env) is a breaking change from around early december. 😃

What's being changed:

Just the example in the docs on how to set and use github env variables.

Check off the following:

@welcome
Copy link

welcome bot commented Jan 2, 2021

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@espipj
Copy link
Contributor Author

espipj commented Jan 2, 2021

I'm happy to help with the Spanish translation of this specific MD file as it seems that it is quite outdated...

@msgirly79

This comment was marked as spam.

@janiceilene janiceilene added 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 ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team labels Jan 4, 2021
@janiceilene
Copy link
Contributor

@espipj Thanks so much for opening a PR! I'll get this triaged for review ✨

I appreciate the offer to help with the translation! Unfortunately, we are not able to accept pull requests for translated content. Our translation process involves an integration with an external service at crowdin.com, where all translation activity happens. We hope to eventually open up the translation process to the open-source community, but we're not there yet.

See https://github.com/github/docs/blob/main/CONTRIBUTING.md#earth_asia-translations for more details.

@janiceilene janiceilene added the localization Issue or PR relating to translation or localization label Jan 4, 2021
@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 12, 2021
@janiceilene
Copy link
Contributor

Thanks for your patience! Our small team is working our way through reviewing all of the amazing contributions ✨

@janiceilene janiceilene removed the stale There is no recent activity on this issue or pull request label Jan 12, 2021
@github-actions
Copy link
Contributor

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 20, 2021
@janiceilene janiceilene removed the ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team label Jan 20, 2021
@skedwards88 skedwards88 self-assigned this Jan 23, 2021
@@ -262,7 +262,19 @@ steps:

### Setting an environment variable

`echo "{name}={value}" >> $GITHUB_ENV`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a great addition! I would recommend replacing the snippet in the "Example" section. That way, users can see the syntax before seeing an example.

So lines 281-287 could instead be something like:

Example

{% raw %}

    steps:
    - name: Set the value
      run: |
          echo "action_state=yellow" >> $GITHUB_ENV
    - name: Use the value
      run: |
          echo $action_state

{% endraw %}

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @skedwards88 actually that's a good idea, I completely missed the example bit. Changing that now :)

Copy link
Contributor

@skedwards88 skedwards88 left a comment

Choose a reason for hiding this comment

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

Looks good! The original example just needs to be deleted now, then I can get this merged down for you.

Comment on lines 265 to 277
{% raw %}
```
steps:
- name: Set the value
id: step_one
run: |
echo "{name}={value}" >> $GITHUB_ENV
- name: Use the value
id: step_two
run: |
echo "${{ env.name }}"
```
{% endraw %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder to delete the example from here now that it is below!

Suggested change
{% raw %}
```
steps:
- name: Set the value
id: step_one
run: |
echo "{name}={value}" >> $GITHUB_ENV
- name: Use the value
id: step_two
run: |
echo "${{ env.name }}"
```
{% endraw %}
`echo "{name}={value}" >> $GITHUB_ENV`

@skedwards88
Copy link
Contributor

🚀 LGTM! I'll get this merged down.

@skedwards88 skedwards88 added the ready to merge This pull request is ready to merge label Jan 29, 2021
@skedwards88 skedwards88 merged commit a4d1c8b into github:main Jan 29, 2021
@github-actions
Copy link
Contributor

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours.

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 localization Issue or PR relating to translation or localization ready to merge This pull request is ready to merge stale There is no recent activity on this issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants