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

fix: use custom version compare function to support multiple version string compare groups #596

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

SteveShani
Copy link
Contributor

@SteveShani SteveShani commented Jun 23, 2024

This fixes an issue where an exception would occur when a python dependency had a version that did not conform to semver specs, such as 1.18.3.9

  • Ready for review
  • Follows CONTRIBUTING rules
  • Reviewed by Snyk internal team

@SteveShani SteveShani self-assigned this Jun 23, 2024
@SteveShani SteveShani requested a review from a team as a code owner June 23, 2024 12:11
@SteveShani SteveShani force-pushed the fix/python-version-comparisons-non-semver branch from 6ef14e7 to d651d71 Compare June 23, 2024 12:40
@SteveShani SteveShani force-pushed the fix/python-version-comparisons-non-semver branch from d651d71 to ead3349 Compare June 24, 2024 07:56
.groups!.VERSION.split(".")
.map((part) => Number.parseInt(part, 10));

const max = v1.length > v2.length ? v1.length : v2.length;
Copy link
Member

@danlucian danlucian Jun 24, 2024

Choose a reason for hiding this comment

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

I would extract the following logic to its function, aiming for a small readability win:

function compareArrays(v1: any[], v2: any[]): number {
  const maxLength = Math.max(v1.length, v2.length);

  for (let i = 0; i < maxLength; i++) {
    const element1 = v1[i] || 0;
    const element2 = v2[i] || 0;

    if (element1 < element2) {
      return 1;
    }

    if (element1 > element2) {
      return -1;
    }
  }

  return 0;
}

The any type should be concrete, just used it for the sake of the example.

What do you say?

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 can do that. To me the function is readable but that's subjective.

Regarding the concrete type, the right answer would be Array[Comparable] but I don't think Typescript has a Comparable type. I'll stick with number or string for now as that's what's likely to appear.

This fixes an issue where an exception would occur when a python dependency had a version that did not conform to semver specs, such as 1.18.3.9
@SteveShani SteveShani force-pushed the fix/python-version-comparisons-non-semver branch from ead3349 to d71bb3f Compare June 25, 2024 09:03
@SteveShani SteveShani merged commit e794601 into main Jun 25, 2024
15 checks passed
@SteveShani SteveShani deleted the fix/python-version-comparisons-non-semver branch June 25, 2024 11:04
@snyk-team-unify
Copy link

🎉 This PR is included in version 6.12.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants