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

building-and-testing-ruby.md - remove wildcard, version, clarify x.0 #13418

Closed
wants to merge 1 commit into from

Conversation

dentarg
Copy link
Contributor

@dentarg dentarg commented Dec 27, 2021

Why:

The wildcard doesn't work in setup-ruby: ruby/setup-ruby#251

What's being changed:

Docs on how to build and test with Ruby on GitHub Actions

Check off the following:

  • I have reviewed my changes in staging (look for "Automatically generated comment" and click Modified to view your latest changes).
  • For content changes, I have completed the self-review checklist.

Writer impact (This section is for GitHub staff members only):

  • This pull request impacts the contribution experience
    • I have added the 'writer impact' label
    • I have added a description and/or a video demo of the changes below (e.g. a "before and after video")

Credits to @MSP-Greg for these changes!

@welcome
Copy link

welcome bot commented Dec 27, 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
Copy link
Contributor

Thanks for submitting a PR to the GitHub Docs project!

In order to review and merge PRs most efficiently, we require that all PRs grant maintainer edit access before we review them. For information on how to do this, see the documentation.

@github-actions
Copy link
Contributor

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 Staging Production What Changed
content/actions/automating-builds-and-tests/building-and-testing-ruby.md Modified Original

@@ -87,13 +87,13 @@ Alternatively, you can check a `.ruby-version` file into the root of your repos

## Testing with multiple versions of Ruby

You can add a matrix strategy to run your workflow with more than one version of Ruby. For example, you can test your code against the latest patch releases of versions 2.7, 2.6, and 2.5. The 'x' is a wildcard character that matches the latest patch release available for a version.
You can add a matrix strategy to run your workflow with more than one version of Ruby. For example, you can test your code against the latest patch releases of versions 3.0, 2.7, and 2.6. YAML parsers will truncate trailing zeros from numeric values, and often convert floats to integers. So, an unquoted 3.0 is converted to an integer, so it will match any minor version, as in 3.0.3, 3.1.0, etc. Likewise, 2.10 needs to be quoted, otherwise it will be interpreted as 2.1.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
You can add a matrix strategy to run your workflow with more than one version of Ruby. For example, you can test your code against the latest patch releases of versions 3.0, 2.7, and 2.6. YAML parsers will truncate trailing zeros from numeric values, and often convert floats to integers. So, an unquoted 3.0 is converted to an integer, so it will match any minor version, as in 3.0.3, 3.1.0, etc. Likewise, 2.10 needs to be quoted, otherwise it will be interpreted as 2.1.
You can add a matrix strategy to run your workflow with more than one version of Ruby. For example, you can test your code against the latest patch releases of versions 3.0, 2.7, and 2.6. The YAML parser used by GitHub Actions will truncate trailing zeros from numeric values, and often convert floats to integers. So, an unquoted 3.0 is converted to an integer, so it will match any minor version, as in 3.0.3, 3.1.0, etc. Likewise, 2.10 needs to be quoted, otherwise it will be interpreted as 2.1.

It is good that we document this gotcha, but it sounds like it is a bug that should be addressed: actions/runner#849 (comment)

Choose a reason for hiding this comment

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

IDK. The text I wrote was meant to not be specific, as most coders would prefer [3.0, 2.7, 2.6] to ['3.0', '2.7', '2.6']. Each item is really an array match to a version string (split into an array), which isn't a number (ie, major.minor.patch, or 3.0.1, worse with alphas, betas, rc's, etc). Hence, I wanted to make readers aware of it without implying a yaml parser is working incorrectly. Not the best explanation, but...

off-topic: For people who don't code Ruby, it's a non-typed language, but 1/3 = 0 and 1/3.0 = 0.333...
Hence, Ruby considers 3 to be an int, and 3.0 to be a float. So, Ruby's default yaml parser considers 3 to be an int, and 3.0 to be a float.

Copy link
Contributor

Choose a reason for hiding this comment

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

Coders may prefer [3.0, 2.7, 2.6] for simplicity, but those aren't numbers, they're strings and they should be quoted. Otherwise you will run into exactly the problem that you describe where 2.10 needs to be quoted to disambiguate from 2.1. I would illustrate the happy path in the doc and then explain why you've quoted all the version numbers instead of trying to tell people when they need to quote.

Copy link

Choose a reason for hiding this comment

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

@ethomson

Thanks. I don't know what's best in this doc, but there are a lot of workflows in the wild that only quote what is needed, ie '3.0'. So, I wanted some background on why... Also, see #15240

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense @MSP-Greg. I think that it's safe to say that mine is an unpopular opinion, so I'd like to give some time to give others a chance to provide feedback, but I think that we should encourage users to quote version numbers and ensure that the starter workflows follow our guidance here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Coders may prefer [3.0, 2.7, 2.6] for simplicity, but those aren't numbers, they're strings and they should be quoted.

I agree with this. The semantic version isn't actually a number, it just works in some cases to treat it as such. Although people often leave out the quotes, we try to craft examples in the docs that illustrate best practices and that will work if users copy-paste-edit without being aware of nuances like this.

@dentarg opened a PR that does this: #15240

@github-actions github-actions bot temporarily deployed to docs-13418--b-t-ruby December 27, 2021 19:16 Inactive
@ramyaparimi ramyaparimi 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 waiting for review Issue/PR is waiting for a writer's review labels Dec 28, 2021
@ramyaparimi
Copy link
Contributor

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

Copy link

@eregon eregon 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 to me 👍

@Mdbevabepari

This comment has been minimized.

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.

Thank you for this improvement! I think it would be best to enclose all of the version numbers in quotes. That will help users who just copy-paste-modify the example without fully understanding the nuance. Then I think the explanation about why can be removed (YAML parsers will truncate...).

(The linked bug seems to concern treating the float 3.0 as the int 3, but even if that was fixed, it wouldn't address 2.10 being treated as 2.1.)

Please let us know if you are willing to make this change!

@skedwards88 skedwards88 added triage Do not begin working on this issue until triaged by the team and removed waiting for review Issue/PR is waiting for a writer's review labels Jan 28, 2022
@skedwards88
Copy link
Contributor

I also opened #14764 to address this more generally

@dentarg
Copy link
Contributor Author

dentarg commented Jan 29, 2022

Please let us know if you are willing to make this change!

While I opened the PR I can't make those changes as the branch is owned by @MSP-Greg , but I'm sure he can make em'

@eregon
Copy link

eregon commented Jan 30, 2022

Note that 2.10 will never exist as a Ruby version, and it's extremely unlikely there would be any X.10.Z version for Ruby.
So Float versions should just work for Ruby, and that's why I decided on this approach early on (it's also what TravisCI supported FWIW).
Unfortunately, the GitHub Actions YAML parser has this known bug actions/runner#849 and there seems to be little effort to fix it.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2022

A stale label has been added to this pull request because it has been open 7 days with no activity. To keep this PR open, add a comment or push a commit within 3 days.

@github-actions github-actions bot added the stale There is no recent activity on this issue or pull request label Feb 6, 2022
@dentarg
Copy link
Contributor Author

dentarg commented Feb 6, 2022

@MSP-Greg are you able to do the requested change (#13418 (review))? If not, I can probably open up a new PR from my fork

@MSP-Greg
Copy link

MSP-Greg commented Feb 6, 2022

@dentarg @skedwards88

I wrote this specifically directed towards Ruby users, and Ruby is often people's first coding language (bootcamps, Rails, etc). Beginners are also unfamiliar with CI systems. I didn't want to imply anything was wrong, just point out differences.

So, I'm happy with the PR.

@github-actions github-actions bot removed the stale There is no recent activity on this issue or pull request label Feb 7, 2022
@dentarg
Copy link
Contributor Author

dentarg commented Feb 8, 2022

@skedwards88 I made the requested changes in #15240, please review that one instead

@skedwards88
Copy link
Contributor

Closing in favor of #15240

@skedwards88 skedwards88 closed this Feb 8, 2022
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 triage Do not begin working on this issue until triaged by the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants