Skip to content

Make null: true and required: true the defaults, update CI Ruby versions#3612

Merged
rmosolgo merged 8 commits into1.13-devfrom
null-defaults
Nov 17, 2021
Merged

Make null: true and required: true the defaults, update CI Ruby versions#3612
rmosolgo merged 8 commits into1.13-devfrom
null-defaults

Conversation

@rmosolgo
Copy link
Copy Markdown
Owner

@rmosolgo rmosolgo commented Sep 8, 2021

Currently, every field must have null: true or null: false. Likewise, every argument must have required: true/required: false. This is verbose, and I think we can recommend a better default. So in this branch:

  • null: true and required: true are applied when those options are omitted. (I picked these because they can always be changed later, unlike null: false or required: false, which require breaking changes to change in the future)
  • A couple rubocop linters to automatically update those configs. (I used the linter to update this codebase, too.)
  • Update the docs
  • As a side-effect, updates this project's rubocop config to 2.5+. I did this so that I could use a recent Rubocop version for writing cops. I think it will be ok to remove support for some ruby/rails versions in 1.13.0, but I may have to keep missing with the build for this to work right.

Does anyone have an opinion on this change? I've done the work to see if it's feasible (it seems to me like it is), and I see the advantage of it, but I'd love to hear any other takes on this idea. (Maybe even the defaults should be different?)

cc @swalkinshaw did you mention once that Shopify applied defaults in these cases?

@rmosolgo rmosolgo added this to the 1.13.0 milestone Sep 8, 2021
@rmosolgo rmosolgo mentioned this pull request Sep 8, 2021
8 tasks
Copy link
Copy Markdown
Contributor

@joelzwarrington joelzwarrington left a comment

Choose a reason for hiding this comment

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

Does anyone have an opinion on this change? I've done the work to see if it's feasible (it seems to me like it is), and I see the advantage of it, but I'd love to hear any other takes on this idea. (Maybe even the defaults should be different?)

Agree with the change, and these defaults make it so that if it needs to change it won't result in a breaking change.

@rmosolgo rmosolgo changed the base branch from master to 1.13-dev November 17, 2021 18:58
@rmosolgo rmosolgo changed the title Make null: true and required: true the defaults Make null: true and required: true the defaults, run CI on Ruby 2.5 Nov 17, 2021
@rmosolgo rmosolgo changed the title Make null: true and required: true the defaults, run CI on Ruby 2.5 Make null: true and required: true the defaults, update CI Ruby versions Nov 17, 2021
@rmosolgo rmosolgo merged commit be2b406 into 1.13-dev Nov 17, 2021
@rmosolgo rmosolgo deleted the null-defaults branch November 17, 2021 20:21
@jgrau
Copy link
Copy Markdown

jgrau commented Dec 6, 2021

For the ones looking for the configuration to run/configure the rubocop cops mentioned in this PR this file has the answer :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants