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

Shell not indicated in some examples, missing alternative shell solutions #253

Closed
Simran-B opened this issue Oct 8, 2020 · 15 comments
Closed
Labels
actions This issue or pull request should be reviewed by the docs actions team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team good first issue Good for newcomers

Comments

@Simran-B
Copy link
Contributor

Simran-B commented Oct 8, 2020

What article on docs.github.com is affected?

For instance, Actions - Workflow commands: Setting an environment variable does not say that the examples are bash specific. They won't work as-is in cmd or powershell on Windows.

What part(s) of the article would you like to see updated?

  • Tell the user what shell an example is for if it's not compatible with all standard terminals
  • Ideally provide examples for other shells as well

Additional information

In Workflow syntax for GitHub Actions there is actually a great example of how it should be:

Examples which indicate intended shell

@welcome
Copy link

welcome bot commented Oct 8, 2020

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

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Oct 8, 2020
@Simran-B
Copy link
Contributor Author

Simran-B commented Oct 8, 2020

On second thought, I think it would make more sense to have tabs similar to the platform specific content, but render the tabs for each example group instead of once at the top of the page:

image

Changing the tab should be reflected by all other shell-specific examples. I'll try to implement a prototype. I think it will require nested Liquid tags? Let me know if you can think of another way.

### About workflow commands

{% shelltabs %}

{% bash %}

`echo "{name}={value}" >> $GITHUB_ENV`

{% endbash %}

{% pwsh %}

`echo "{name}={value}" | Out-File -Append -FilePath $env:GITHUB_ENV -Encoding utf8`

{% endpwsh %}

{% cmd %}

`chcp 65001 > nul & echo {name}={value} >> %GITHUB_ENV%`

{% endcmd %}

{% endshelltabs %}

@zeke zeke added the windows label Oct 8, 2020
@Simran-B
Copy link
Contributor Author

Simran-B commented Oct 8, 2020

Hard-coding it to shells might actually allow this to be implemented without nesting tags, but I'm in favor of a generic tab widget solution. I believe that it will require a group name to correctly update related example groups, and to avoid switching tabs of unrelated groups with an equally named tab. Something along:

{% tabs "shell" %}
{% tab "Bash" %} ... {% endtab %}
{% tab "PowerShell" %} ... {% endtab %}
{% tab "Windows `cmd`" %} ... {% endtab %}
{% endtabs %}

@janiceilene janiceilene added actions This issue or pull request should be reviewed by the docs actions team ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team labels Oct 8, 2020
@janiceilene
Copy link
Contributor

Thanks for opening an issue @Simran-B! I'll let the @github/docs-content-ecosystem review 💛

@lucascosti lucascosti added enhancement good first issue Good for newcomers and removed triage Do not begin working on this issue until triaged by the team labels Nov 2, 2020
@lucascosti
Copy link
Contributor

Thanks a lot for these suggestions, @Simran-B!

  • Tell the user what shell an example is for if it's not compatible with all standard terminals
  • Ideally provide examples for other shells as well

Although Linux/bash is our default when showing examples (the vast majority of Actions CI is done using Linux runners), we can do a lot better at explaining and showing this in our examples.

On second thought, I think it would make more sense to have tabs similar to the platform specific content, but render the tabs for each example group instead of once at the top of the page

This is a good idea, but I don't think we have plans at the moment to expand the 'tabs' we have in the documentation. This will require some engineering effort to support additional liquid tags, and it's not currently on our roadmap. At the moment, we only support operating system tags.

I've triaged this as an enhancement issue, so anyone can pick up and add explanatory text/headings to examples that require some explanation on the appropriate shell/OS 🙂

@Simran-B
Copy link
Contributor Author

Simran-B commented Nov 2, 2020

@lucascosti Thanks for the positive feedback!

This will require some engineering effort to support additional liquid tags, and it's not currently on our roadmap.

I'm looking into an implementation of general purpose tabs, but I'm uncertain about several aspects of the documentation system and hit a few problems. Is there anyone I can talk to about the architecture and the components of the system? There seems to be very little documentation about the documentation system. Shopify's liquid implementation in Ruby and its documentation is somewhat helpful, but I'm unsure how closely liquid.js follows its design.

@lucascosti
Copy link
Contributor

Is there anyone I can talk to about the architecture and the components of the system?

I'll tag in @github/docs-engineering for this, and if they're open to receiving contributions 🙂

@himanshu211raj
Copy link

Is there anyone I can talk to about the architecture and the components of the system?

I'll tag in @github/docs-engineering for this, and if they're open to receiving contributions 🙂

I am here and now I am always ready to contribute in open source. But due to the reason that I am a beginner so I have limited knowledge and day by day I will improve my skills to contribute more and more to open source.

@Simran-B
Copy link
Contributor Author

Simran-B commented Nov 25, 2020

I have a semi-functional prototype for a generic tab widget here if you are interested: #1606

@ArchieCoder
Copy link

@lucascosti This year, I spent a lot of times starting from scratches multiple Windows workflows and let me tell you that it was a very painful experience since the doc uses Linux as first class citizen versus Windows as third class citizen. Here is an important page which should cover all OSes: https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions#github-context. This is the basic about using GitHub Actions. The GitHub Actions from the marketplace is also another painful experience since a bunch of them just does not work with Windows and we don't know until we try it. The whole experience is not great for Windows developers.

@lucascosti
Copy link
Contributor

Hi @ArchieCoder! Thanks for this feedback. Because of resource constraints, we generally focus on the most common use-case in our docs as a default, which are Linux runners. However, we can do a better job providing Windows examples where it's needed.

Here is an important page which should cover all OSes: https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions#github-context. This is the basic about using GitHub Actions.

🤔 I'm a bit confused by this. The only Linux-specific content that I can see on that page is the Linux syntax for echoing an environment variable, e.g. run: echo $<something>. IIUC, almost everything else shown there on context and expressions should be the same in a workflow file regardless of the operating system used as a runner. Is there something specific there that won't work on Windows?

We are of course open to receiving contributions for improving our content for Windows developers. 🙂

The GitHub Actions from the marketplace is also another painful experience since a bunch of them just does not work with Windows and we don't know until we try it. The whole experience is not great for Windows developers.

Yes, although most GitHub-developed actions should work fine on Windows, how an action works is up to its developer. Most actions are not developed by GitHub, and I would suggest interacting and collaborating with an action's developer through their action's repo.

If you'd like to discuss this further, I'd suggest opening a docs discussion or starting a conversation in the GitHub Support Community so other members of the community can also discuss 🙂

@ArchieCoder
Copy link

@lucascosti Hi, Okay, you are right that the Context and expression syntax page is okay! I apologize.

I posted the wrong page https://docs.github.com/en/free-pro-team@latest/actions/reference/environment-variables.

image

@Simran-B
Copy link
Contributor Author

Simran-B commented Dec 15, 2020

I opened a PR to add env var examples for shells other than Bash: #2121
I'll add a couple more examples to other pages.

A couple of questions:

  1. What is the desired naming scheme in headline?

    • Example using bash or Example using Bash or Example using `bash`
    • Example using PowerShell or Example using PowerShell Core or Example using pwsh or Example using `pwsh`
    • Example using Windows `cmd` or Example using Cmd.exe or Example using Command Prompt or something else
  2. What would be the desired tab labels, should Tab widget (prototype) #1606 land?
    (Probably the same as above without Example using, but whether or not <code> (e.g. Windows `cmd`) shall be used there is unclear)

  3. There is a list of shells - should there be an example for each (ideally), or should it be limited to bash, pwsh and cmd?

  4. If many cases, the same example code will work in both PowerShell and PowerShell Core. But there are corner cases, in particular around character encoding and BOM handling, where classic PowerShell requires more cumbersome commands. Since PowerShell Core is the default Windows shell in GitHub-hosted runners, are pwsh examples preferred (also see 2.) and should the PowerShell Core variant use the most simple commands - or - should they be treated as the same, and thus one example work in both, even if it's more complicated than necessary for pwsh ? Should aliases be avoided, e.g. use Write-Output instead of echo?

  5. Should PowerShell Core examples use backslashes in paths, even though it's not Windows exclusive? Should a drive letter be included? Example: path\to\dir vs. X:\path\to\dir

  6. Can Python examples assume Python 3, maybe even Python 3.7/3.8? Should they normalize paths to work cross-platform? What about drive letters however? Example: os.path.normpath('X:/path/to/dir') (Cygwin style paths a la /c/... aren't normalized to C:\... by Python)

  7. Should examples explicitly set shell: ... in steps, even if it's not required in some cases?

  8. What if e.g. Bash and PowerShell examples are identical? There isn't much value in repeating examples at the moment. One could change the headline to Example using Bash / PowerShell Core to avoid duplication. However, with tabs to select the shell one is interested in, it would be much better to have an example for each (and I see Actions: Add examples for shells other than Bash #2121 as a preparation for that).

  9. Any suggestions on how to unclutter the headlines here?
    Adding more examples will add several more headlines.
    Current structure:

    • <h3> Setting an environment variable
      • <h4> Example
      • <h4> Multiline strings
        • <h5> Example
  10. How to handle the case where an example description is present? Should the description be repeated for each shell example?

    #### Example masking a string
    
    When you print `"Mona The Octocat"` in the log, you'll see `"***"`.
    
    ```bash
    echo "::add-mask::Mona The Octocat"
    ```
    

@janiceilene
Copy link
Contributor

We haven't had much activity on this issue in a while and it seems to be collecting spam. I'm going to close it now, but if anyone wants to pick work back up, we can absolutely reopen it! Thanks!

jnidzwetzki pushed a commit to jnidzwetzki/docs that referenced this issue Oct 6, 2022
* Page setup

* Edit initial content

* Rename the file to remove MST

* Apply suggestions from code review

Co-authored-by: Ryan Booz <ryan@timescale.com>

* Apply suggestions from code review

Co-authored-by: Ryan Booz <ryan@timescale.com>
Co-authored-by: Jacob Prall <prall.jacob@gmail.com>
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 ecosystem This issue or pull request should be reviewed by the Docs Ecosystem team good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

11 participants
@zeke @ArchieCoder @lucascosti @Simran-B @janiceilene @himanshu211raj and others