Skip to content

Conversation

@steven-wolfman
Copy link
Contributor

@steven-wolfman steven-wolfman commented Apr 6, 2021

Why:

Closes #5103

What's being changed:

The syntax $default-branch is specific to workflow templates only and cannot be used in standard workflows. As a result, a copy-paste of the existing yaml workflow for "Starting with the Node.js workflow template" will fail. See steven-wolfman/test-workflow-template-syntax@0d006c5#diff-fbdccf80e37a6640cb78bdcf9d90597cb91421747fd5c2365d878287e744ff99 for an example commit that triggers a workflow using the main syntax but fails to trigger workflows using the $default-branch syntax.

Check off the following:

Drafting my PR template.

### Why:

**Closes github#5103

### What's being changed:

[The syntax `$default-branch` is specific to workflow templates only and cannot be used in standard workflows.](https://github.com/github/docs/discussions/5071) As a result, a copy-paste of the existing yaml workflow for "Starting with the Node.js workflow template" will fail. See steven-wolfman/test-workflow-template-syntax@0d006c5#diff-fbdccf80e37a6640cb78bdcf9d90597cb91421747fd5c2365d878287e744ff99 for an example commit that triggers a workflow using the `main` syntax but fails to trigger workflows using the `$default-branch` syntax.

### Check off the following:
- [ ] I have reviewed my changes in staging. (look for the **deploy-to-heroku** link in your pull request, then click **View deployment**)
- [x] For content changes, I have reviewed the [localization checklist](https://github.com/github/docs/blob/main/contributing/localization-checklist.md)
- [x] For content changes, I have reviewed the [Content style guide for GitHub Docs](https://github.com/github/docs/blob/main/contributing/content-style-guide.md).
@welcome
Copy link

welcome bot commented Apr 6, 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.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Apr 6, 2021
@steven-wolfman steven-wolfman marked this pull request as ready for review April 6, 2021 19:17
{% data variables.product.prodname_dotcom %} provides a Node.js workflow template that will work for most Node.js projects. This guide includes npm and Yarn examples that you can use to customize the template. For more information, see the [Node.js workflow template](https://github.com/actions/starter-workflows/blob/main/ci/node.js.yml).

To get started quickly, add the template to the `.github/workflows` directory of your repository.
To get started quickly, add the template to the `.github/workflows` directory of your repository. (If your default branch is not named `main`, change `main` to your default branch's name.)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd love feedback from a writer on how and whether to comment on what to do for people whose default branch is not named main.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whatever text ends up being used, the same text should appear in the Ruby version also changed in this commit.

@steven-wolfman
Copy link
Contributor Author

steven-wolfman commented Apr 6, 2021

OK. There are a few more places where this appears. In English:

Plus these translations of both the Ruby and Node.js documents:

(A grep of all files for $default-branch shows only these locations plus appropriate reference at https://github.com/steven-wolfman/docs/blob/main/content/actions/learn-github-actions/sharing-workflows-with-your-organization.md and its translations.)

I'll work on an update for https://github.com/steven-wolfman/docs/tree/patch-1/content/actions/guides/building-and-testing-ruby.md. I don't think I can reasonably help with the text in any of the translations.

Exactly parallel changes. Should be kept in sync with the Node.js changes.
Copy link

@Nirzak Nirzak left a comment

Choose a reason for hiding this comment

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

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

@steven-wolfman
Copy link
Contributor Author

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

I agree that it is not compulsory to name the default branch main.

The trouble is that the existing code shows $default-branch which does not work no matter what your default branch is named. Instead, that syntax is specific to GitHub workflow templates and does not work in standard GitHub workflows.

I'm unsure whether putting main into the raw code is the right way to go, but leaving it as $default-branch with no further commentary seems clearly wrong.

Does that make sense?

@Nirzak
Copy link

Nirzak commented Apr 6, 2021

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

I agree that it is not compulsory to name the default branch main.

The trouble is that the existing code shows $default-branch which does not work no matter what your default branch is named. Instead, that syntax is specific to GitHub workflow templates and does not work in standard GitHub workflows.

I'm unsure whether putting main into the raw code is the right way to go, but leaving it as $default-branch with no further commentary seems clearly wrong.

Does that make sense?

Yeah I agree but for that an elaborated documentation is needed. If we just put main in the raw code then many one will think they have to set main in their workflow. So the people who have default-branch named as "master" will face error.

@steven-wolfman
Copy link
Contributor Author

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

I agree that it is not compulsory to name the default branch main.

The trouble is that the existing code shows $default-branch which does not work no matter what your default branch is named. Instead, that syntax is specific to GitHub workflow templates and does not work in standard GitHub workflows.

I'm unsure whether putting main into the raw code is the right way to go, but leaving it as $default-branch with no further commentary seems clearly wrong.

Does that make sense?

Just to make this more concrete, please do open steven-wolfman/test-workflow-template-syntax@0d006c5

There, you can see that a workflow that directly copies-and-pastes the $default-branch syntax will not run on a push to the default branch. (Click the green checkmark to see what did run, which was the only one of the three workflows that uses main. The two workflows using $default-branch didn't run because the push was not to a branch named literally $default-branch. You can see the copy-pasted workflow that uses $default-branch at https://github.com/steven-wolfman/test-workflow-template-syntax/blob/0d006c52a904911ddb71a13994a4864d7d022e1c/.github/workflows/copy-paste-of-nodejs-template.yml. All of this would work the same with source as the default branch, replacing main where it appears, i.e., only one of the three workflows would run.)

@steven-wolfman
Copy link
Contributor Author

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

I agree that it is not compulsory to name the default branch main.
The trouble is that the existing code shows $default-branch which does not work no matter what your default branch is named. Instead, that syntax is specific to GitHub workflow templates and does not work in standard GitHub workflows.
I'm unsure whether putting main into the raw code is the right way to go, but leaving it as $default-branch with no further commentary seems clearly wrong.
Does that make sense?

Yeah I agree but for that an elaborated documentation is needed. If we just put main in the raw code then many one will think they have to set main in their workflow. So the people who have default-branch named as "master" will face error.

Totally, but isn't that better than the current case where everyone will have an error?

And.. is the correct fix for the more elaborate documentation to clarify the need for rename just before the code block (which is what I tried)?

Thanks!

@Nirzak
Copy link

Nirzak commented Apr 6, 2021

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

I agree that it is not compulsory to name the default branch main.
The trouble is that the existing code shows $default-branch which does not work no matter what your default branch is named. Instead, that syntax is specific to GitHub workflow templates and does not work in standard GitHub workflows.
I'm unsure whether putting main into the raw code is the right way to go, but leaving it as $default-branch with no further commentary seems clearly wrong.
Does that make sense?

Yeah I agree but for that an elaborated documentation is needed. If we just put main in the raw code then many one will think they have to set main in their workflow. So the people who have default-branch named as "master" will face error.

Totally, but isn't that better than the current case where everyone will have an error?

And.. is the correct fix for the more elaborate documentation to clarify the need for rename just before the code block (which is what I tried)?

I think it's not compulsory to rename your default branch main. I have set "source' as my default branch and I have written the workflow as on: push: branches: [source] and the code just works fine for me. So, default-branch option is the correct one. It shouldn't be stated as 'main' as people can rename their default branch to any name.

I agree that it is not compulsory to name the default branch main.
The trouble is that the existing code shows $default-branch which does not work no matter what your default branch is named. Instead, that syntax is specific to GitHub workflow templates and does not work in standard GitHub workflows.
I'm unsure whether putting main into the raw code is the right way to go, but leaving it as $default-branch with no further commentary seems clearly wrong.
Does that make sense?

Yeah I agree but for that an elaborated documentation is needed. If we just put main in the raw code then many one will think they have to set main in their workflow. So the people who have default-branch named as "master" will face error.

Totally, but isn't that better than the current case where everyone will have an error?

And.. is the correct fix for the more elaborate documentation to clarify the need for rename just before the code block (which is what I tried)?

Thanks!

Yeah default-branch must have to be rename as the branch name that you've set as your default-branch. Set it as main in the raw code will help many ones because we know a huge number of github users are using main as their default-branch. But it still there should be extra documentation for those who have custom default-branch.

@janiceilene
Copy link
Contributor

Thanks so much for opening an issue and a PR @steven-wolfman! You don't need to worry about any of the translations, updated content in English is automatically sent to our translators once it's merged. I'll get this triaged for review ⚡

@janiceilene janiceilene added actions This issue or pull request should be reviewed by the docs actions team waiting for review Issue/PR is waiting for a writer's review and removed triage Do not begin working on this issue until triaged by the team labels Apr 8, 2021
@steven-wolfman
Copy link
Contributor Author

steven-wolfman commented Apr 8, 2021 via email

@lucascosti lucascosti self-assigned this Apr 13, 2021
Copy link
Contributor

@lucascosti lucascosti left a comment

Choose a reason for hiding this comment

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

Thanks for this, @steven-wolfman! I slightly reworded the mentioning of main before the workflow, and replaced a few other instances of $default-branch in the ruby article.

I'll get this merged in! 🎉

@lucascosti lucascosti merged commit 92b3b22 into github:main Apr 13, 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. If you're looking for your next contribution, check out our help wanted issues

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 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.

Correct use of $default-branch syntax for an example of a standard (non-template) workflow

4 participants