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

feat(resolutions): support resolutions ignoring until given date #291

Conversation

padamczewski
Copy link
Contributor

📝 Description

Why?

Even though we have the capability to exclude some packages included in resolutions field from validation it doesn't cater for a case where we know ahead of time that we would like to be notified about pinning dependency eventually at some point in the future. Example that comes to mind:

  • I notice issue with a package today, raise a PR knowing the desire is to get rid of that specific resolution
  • I'd like to set until when the ignoring of resolutions can last
  • The actions starts throwing when the set date is passed

What?
This PR brings in additional resolutions related config in form of single line string ignore-resolutions-until, which is accepting a date as an input.

How?

  • Ignore resolutions when they are present and ignore date is in the future: code and passing-test
  • Hit the default failure when resolutions exist, but date is in the past passing-test
  • Respect ignore-resolutions when they match resolutions even though the date is in past passing-test

🔗 Related Issues

@@ -15,8 +15,14 @@ import * as core from '@actions/core';
import { PackageJson } from 'type-fest';

export const validateResolutions = (packageJson: PackageJson) => {
const ignoredResolutions = core.getMultilineInput('ignore-resolutions');
const ignoreUntilDate = new Date(core.getInput('ignore-resolutions-until'));
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when core.getInput('ignore-resolutions-until') returns undefined? I think we should explicitly handle the case when this input is not provided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's actually a good shout. Have updated the logic and added a relevant test here.

Not a huge fan of nested if statements, but feels fairly all right for this use case.

@padamczewski padamczewski force-pushed the feat/add-resolution-ignoring-by-date branch from 2dd0c08 to 18f473f Compare May 4, 2024 15:50
@danadajian danadajian merged commit 8c7543c into ExpediaGroup:main May 5, 2024
3 checks passed
@danadajian
Copy link
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on GitHub release

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.

2 participants