we have no violations (likely because we upgraded to Ruby 3)
Change-Id: I64242e28afaeff72e7fbe7b972e5e9bdc1d84884
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/341936
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
[skip-crystalball]
and apply updated copys (RSpec/Eq and RSpec/MetadataStyle, and one
instance that the split RSpec/SpecFilePathSuffix caught)
Change-Id: I3872458f35b791f1ce3f8108a2aaf4fff2cfd612
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/327204
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
Build-Review: Isaac Moore <isaac.moore@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
google-api-client is deprecated and unmaintained; need to use
the service specific gem. this gets us on the latest APIs, and
importantly unblocks us on updating other common dependencies
(most notably a step towards updating faraday, but also retriable)
Change-Id: I646da7dc68b8c5f6068142608c19771dafbef127
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/325392
QA-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
it's been split to its own gem (still implicitly depedended on by
rubocop-rspec), and we do use factory_bot, so make sure it's getting
checked.
Change-Id: Ia49296b561f59a1503ae097535c404c1c1d5572d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/320103
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
manually done. the big one was the explicit locale assignment
in set_locale_with_localizer needs undone in a controller callback
then using with_locale everywhere, specs no longer need to be concerned
about being in an uncertain locale
Change-Id: I5a1d2c907a6f52ee4d8c2307b8c789a1f1ea436e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/320112
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
Change-Id: I6abefdfa9fed6dd4525c8786e93efa548b3710f2
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/319603
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jacob Burroughs <jburroughs@instructure.com>
Product-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Migration-Review: Jacob Burroughs <jburroughs@instructure.com>
refs AE-297
and also include binstubs in the path in docker
this will help ensure people are running the correct version of bundler
Change-Id: I68e7f6a43efd89ff257a9307aa4f6709acef05f8
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/318776
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
We're firmly on Rails 6.1+; no need to worry about legacy behavior
Change-Id: I61374cba137b340a9c9792f46898bf2179c3c62d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/318969
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
and remove now-duplicated config
also fix a few offenses that snuck in due to code review race conditions
as part of this, Rubocop was loading the lockfile, so our "clever" rubocop
binstub had to be adjusted to use a lockfile. this allows us to commit the
lockfile, which I then also updated the sync_lockfiles script to handle
this one as well
flag = none
test plan: N/A
Change-Id: I20ee2672c5219a40e90bc1138f8900a8d2ce08ee
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/317373
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
in case you have some override gems installed, don't worry about them
Change-Id: I9f084621322eb834fc1774804bad6e433861625b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/316059
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Shafovaloff <ashafovaloff@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
I was able to replace one of our custom cops with a built in one
(just had to make it ignore older migrations).
Then I had to manually fix a couple of (important!) offenses
Change-Id: I000310bb6b065034384ba3a33ef5e37e22b9be5a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/315855
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
includes some minor hash layout fixes that were incompatible with prior rubocop
[skip-stages=Flakey]
Change-Id: I75e903292daa70c84b03600b97fac49ca1155004
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/315786
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Jacob Burroughs <jburroughs@instructure.com>
Build-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
refs AC-1
Overriding default_scope here causes statement caching to never be used, so replace it with a rubocop linter.
Change-Id: Ifbe84a87ab19fc2f6032ed982fcbe8019e765dd1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/305925
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Aaron Ogata <aogata@instructure.com>
Product-Review: Aaron Ogata <aogata@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
because this isn't even pointless pedantry. it's just pointless
Change-Id: I7b8cd656a4adb8da3b5e30a59593297f91d9aa9a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/286846
Reviewed-by: Aaron Ogata <aogata@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
seems overly pedantic
Change-Id: Ibe3ccd76978650842dd09fd1232df5673e292d58
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/287118
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
to allow the modifier form, e.g. this is okay
if condition_a || condition_b
do_something unless condition_c && condition_d
end
and is in fact preferable to
if (condition_a || condition_b) && !(condition_c && condition_d)
do_something
end
which is what rubocop would turn it into otherwise
Change-Id: I1b917e3ea5149379f22e70294d263b713b3b9b35
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282475
Reviewed-by: Eric Saupe <eric.saupe@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
because it gives bad advice. `str.casecmp("foo").zero?` is not only
less clear than `str.downcase == "foo"`, but it is not Unicode-aware
(ironically, `str.casecmp?("foo")`, with the question mark, _is_
Unicode-aware)
Change-Id: I1abd17f4a09b205073998c5cf9e89cb4bd85ad6b
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282294
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
test plan:
- create a migration (locally) and add it to a commit
(no need to actually commit, push, or even run the migration)
that adds a foreign key to an existing table, e.g.
add_foreign_key :users, :eportfolios
- run script/rlint, and it should complain that you should
use a non-transactional migration and add `delay_validation: true`
to the arguments
- do those things, run script/rlint again, and it should not
complain
- add a foreign key to a table created in the same migration,
e.g.
create_table :foo do |t|
end
add_foreign_key :foo, :bar
- run script/rlint again, and it shouldn't complain
flag=none
closes DE-940
Change-Id: I3f235cbf1ff4011e20d74bdaa8953b857c7e330a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280514
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
since planned cops may provide different severity levels
depending on circumstances
refs DE-942
flag = none
Change-Id: I7501c13ea350241c47debceb7440cc8ce3d43925
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/280532
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Isaac Moore <isaac.moore@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
it's removing disable directives that need to be there
Change-Id: Iecff7d5559742af834946a6d9223fd24634baa90
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279693
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
manual. obvious duplicates were removed. others I looked for context on what
the original copy/paste error was.
Change-Id: I6d009c480eacf9233ed0c8253ce715bc2f74e8bf
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279432
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[skip-stages=Flakey]
manual, mostly by fixing copy/paste name problems,
combining groups that didn't need separating, or creating
new groups to contain contiguous top level groups that can't
be combined due to having different setup
Change-Id: I5b21664eabd8c3a62e22c09a28ba8bf519371aa5
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279428
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
means we can remove a whole bunch of specific cop configurations that
now inherit the proper settings. cops that still have violations need
to be explicitly named to make them not error. this means that the
list of remaining cops is explicitly in our config now. also combined
the don't-auto-correct lists with the not-yet-fixed lists, so that
it's only a "permanent" section and a "todo" section
the final computed configuration is unchanged by this commit, with one
exception - Bundler/OrderedGems was moved to the Gemfile.d/.rubocop.yml
override, and some of the internal gems violated that so their
gemfiles have been autocorrected
Change-Id: I100884a96dbe9eb9d4bfc0692663c271daa9de16
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279413
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
we're far enough along that the generated disablings are no longer useful
Change-Id: Iec341384f7a5f6eeb42bad4ae1ea12f433d270a4
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/279397
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
it was just too confusing on which one an editor is using, double comments
in jenkins, etc.
this is accomplished by several things:
* required cops are just marked as severe, instead of using a separate
config for them, and failing if anything shows up from that config
* get rid of all the logic to only include certain directories for
certain cops. turns out it's not _that_ ominous to correct errors
across the entire repository before marking a cop as required.
* but still auto-generate config to turn _off_ autocorrect for
non-severe cops. this is important because auto-correct must run
for entire files, and we don't want it auto-correcting optional
things that you didn't touch.
* update gergich to get more details from the parsed comments.
this plus the prior point means we _don't_ have to have heavy mode when
in autocorrecting, but we still display out-of-context lines that were
autocorrected
this also makes it so we can use per-dir .rubocop.yml files again, so
take some of the exceptions out of the root and put them in their own
directory
Change-Id: Ie936d1a9920b68910acd250ba817c7b4a670b958
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274394
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Cody Cutrer