dry-run: generate ignored versions from "update-types" #3513

thepwagner · 2021-04-19T15:35:40Z

Continuing #3512 and #3525 , this migrates Dependabot::Config::UpdateConfig::IgnoreCondition to Dependabot::Config::IgnoreCondition, and amends the #ignored_versions() function to support dynamically ignored version ranges, as specified by #2219 (comment) .

This iteration supports the functionality via the dry-run.rb script, 🎉 . Dependabot doesn't yet recognize the syntax, but it will parse a .github/dependabot.yaml configuration file:

updates:
  - package-ecosystem: "npm"
    directory: "/"
    schedule:
      interval: "monthly"
    ignore:
      - dependency-name: "lodash"
        update-types: ["version-update:semver-major"]

On Wed, 21 Apr 2021 at 17:41, Pete Wagner ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In common/lib/dependabot/config/ignore_condition.rb <#3513 (comment)> : > + private + + def versions_by_type(dep) + case @update_types.first # FIXME: flatmap + when :ignore_patch_versions + [ignore_version(dep.version, 4)] + when :ignore_minor_versions + [ignore_version(dep.version, 3)] + when :ignore_major_versions + [ignore_version(dep.version, 2)] + else + [] + end + end + + def ignore_version(version, precision) That link 404s for our external friends :/ I think the equivalent "a" for pre-release is on L46, I'll include pre-release considerations in the test suite. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3513 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAE5RPOUNQSGJLB5WBHCBDTJ3545ANCNFSM43GCOA3A> .

thepwagner

I marked RFR as this is code complete and roughly what I was going for.

As noted, the way conditions are aggregated needs some attention:

What if there are multiple matches with identical update-types:?
What if there are multiple matches with different update-types:?
What if there are update-types: and versions:?

This definitely isn't shippable, but is a worthy iteration.

common/lib/dependabot/config/ignore_condition.rb

thepwagner · 2021-04-21T18:18:34Z

common/lib/dependabot/config/update_config.rb

-          select { |ic| ic.dependency_name == dep.name }. # FIXME: wildcard support
-          map(&:versions).
+          select { |ic| ic.dependency_name == dependency.name }. # FIXME: wildcard support
+          map { |ic| ic.ignored_versions(dependency) }.


Concern: as I as alluding to in https://github.com/dependabot/dependabot-core/pull/3536/files#r617772124 , this isn't going to behave when multiple conditions match the target dependency...

Not sure I follow, do we have a test case for this that will fail currently?

I was looking at moving the dependency name checking into the IgnoreCondition class but would mean returning an empty ignored_versions if the dependency name doesn't match the ic.dependency_name which seems confusing to me but unsure if I'm missing something else?

this isn't going to behave when multiple conditions match the target dependency

Is this case tested here?: https://github.com/dependabot/dependabot-core/pull/3513/files#diff-1bee1e376554d0d150b3a22f7b1ca87ac8d9de442cebf942c2f0e3f7fcc50c04R56

common/lib/dependabot/config/ignore_condition.rb

common/spec/dependabot/config/ignore_condition_spec.rb

…e-type

feelepxyz · 2021-04-22T15:12:45Z

common/spec/dependabot/config/ignore_condition_spec.rb

+        ignored = reqs.any? { |req| req.satisfied_by?(version) }
+        expect(ignored).to eq(true), "Expected #{v} to be ignored, but was allowed"
+      end
+    end


Nice test helpers 👍

common/lib/dependabot/config/ignore_condition.rb

feelepxyz

This is looking good! Tests are real clear and easy to follow 💯

I've been testing this on dry-runs and it's working well 🎉 Should we ship this and start incorporating in updater? Not sure if it's worth refactoring the wildcard matching as I see it possibly being more confusing having the dependency name filtering done in the individual IgnoreCondition but might have missed some edge-case that's not working.

feelepxyz · 2021-04-22T15:57:24Z

What if there are multiple matches with identical update-types:?

What if there are multiple matches with different update-types:?

What if there are update-types: and versions:?

Tried to add some tests around this. Added a uniq on the generated version requirements but seems to work? 🤔

feelepxyz

I've been testing a bunch of dry-runs with different ignore setups and looking good so far! 🚢

This was a thing before the refactor - it was replaced with `try { Integer(x) } catch (ArgumentError e) {}`, to handle this case. Co-authored-by: Philip Harrison <philip@mailharrison.com>

thepwagner self-assigned this Apr 19, 2021

Base automatically changed from dry-run-config-file to main April 20, 2021 14:37

feelepxyz reviewed Apr 20, 2021

View reviewed changes

common/lib/dependabot/config.rb Outdated Show resolved Hide resolved

thepwagner force-pushed the relative-ignore-update-type branch from f4e9669 to 80b93ec Compare April 20, 2021 17:34

feelepxyz mentioned this pull request Apr 20, 2021

Add ignore conditions to update checker #3521

Closed

thepwagner mentioned this pull request Apr 20, 2021

UpdateConfig ruby-er model #3525

Merged

IgnoreCondition: update-type

7375421

thepwagner force-pushed the relative-ignore-update-type branch from 80b93ec to 7375421 Compare April 20, 2021 19:08

thepwagner changed the base branch from main to update-config-poro April 20, 2021 19:08

Base automatically changed from update-config-poro to main April 21, 2021 13:13

feelepxyz reviewed Apr 21, 2021

View reviewed changes

common/lib/dependabot/config/file.rb Outdated Show resolved Hide resolved

thepwagner added 5 commits April 21, 2021 09:48

Merge remote-tracking branch 'origin/main' into relative-ignore-updat…

6bf1e69

…e-type

plural update_types

7523f70

extract Dependabot::Config::IgnoreCondition

ce731b3

ConfigFile - parse ignore update-types:

1fd716e

TDD dynamic ignore conditions

4d6a275

feelepxyz reviewed Apr 21, 2021

View reviewed changes

common/spec/dependabot/config/ignore_condition_spec.rb Outdated Show resolved Hide resolved

feelepxyz reviewed Apr 21, 2021

View reviewed changes

common/lib/dependabot/config/ignore_condition.rb Outdated Show resolved Hide resolved

thepwagner added 2 commits April 21, 2021 13:26

TDD: green

6a4250d

IgnoreConditionSpec

7246f96

ignores on non-semver dependencies

4d10019

thepwagner changed the title ~~experiment: generate ignored versions from "update-type"~~ experiment: generate ignored versions from "update-types" Apr 21, 2021

thepwagner mentioned this pull request Apr 21, 2021

UpdateConfig: Match wildcards in dependency names #3536

Merged

thepwagner marked this pull request as ready for review April 21, 2021 18:10

thepwagner requested a review from a team as a code owner April 21, 2021 18:10

thepwagner commented Apr 21, 2021

View reviewed changes

mctofu reviewed Apr 21, 2021

View reviewed changes

thepwagner added 2 commits April 22, 2021 07:13

TDD: refined ignore contract

690d37a

desired dynamic ignore behaviour

55975f9

thepwagner added 2 commits April 22, 2021 09:56

Merge remote-tracking branch 'origin/main' into relative-ignore-updat…

ab1dc39

…e-type

appease linter

509853f

feelepxyz reviewed Apr 22, 2021

View reviewed changes

common/lib/dependabot/config/ignore_condition.rb Outdated Show resolved Hide resolved

feelepxyz reviewed Apr 22, 2021

View reviewed changes

Add tests for update types

36a45fc

feelepxyz approved these changes Apr 22, 2021

View reviewed changes

thepwagner and others added 2 commits April 22, 2021 13:06

Remove dead code

23c97fc

This was a thing before the refactor - it was replaced with `try { Integer(x) } catch (ArgumentError e) {}`, to handle this case. Co-authored-by: Philip Harrison <philip@mailharrison.com>

IgnoreCondition: test shorter semver versions+fix

0e44f02

thepwagner changed the title ~~experiment: generate ignored versions from "update-types"~~ generate ignored versions from "update-types" Apr 22, 2021

thepwagner changed the title ~~generate ignored versions from "update-types"~~ dry-run: generate ignored versions from "update-types" Apr 22, 2021

thepwagner merged commit c575aee into main Apr 22, 2021

thepwagner deleted the relative-ignore-update-type branch April 22, 2021 18:38

This was referenced Apr 22, 2021

v0.143.2 #3545

Merged

IgnoreCondition: severity #3552

Closed

nan-wang mentioned this pull request May 4, 2021

Depedency updates should NOT take place for each patch release jina-ai/examples#548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dry-run: generate ignored versions from "update-types" #3513

dry-run: generate ignored versions from "update-types" #3513

thepwagner commented Apr 19, 2021 •

edited

Loading

feelepxyz commented Apr 21, 2021 via email

thepwagner left a comment

thepwagner Apr 21, 2021

feelepxyz Apr 22, 2021

feelepxyz Apr 22, 2021

feelepxyz Apr 22, 2021

feelepxyz left a comment

feelepxyz commented Apr 22, 2021

feelepxyz left a comment

dry-run: generate ignored versions from "update-types" #3513

dry-run: generate ignored versions from "update-types" #3513

Conversation

thepwagner commented Apr 19, 2021 • edited Loading

Related

feelepxyz commented Apr 21, 2021 via email

thepwagner left a comment

Choose a reason for hiding this comment

thepwagner Apr 21, 2021

Choose a reason for hiding this comment

feelepxyz Apr 22, 2021

Choose a reason for hiding this comment

feelepxyz Apr 22, 2021

Choose a reason for hiding this comment

feelepxyz Apr 22, 2021

Choose a reason for hiding this comment

feelepxyz left a comment

Choose a reason for hiding this comment

feelepxyz commented Apr 22, 2021

feelepxyz left a comment

Choose a reason for hiding this comment

thepwagner commented Apr 19, 2021 •

edited

Loading