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

Make licensed status detect dependencies that have been removed #656

Closed
Mathbl opened this issue May 24, 2023 · 9 comments · Fixed by #657
Closed

Make licensed status detect dependencies that have been removed #656

Mathbl opened this issue May 24, 2023 · 9 comments · Fixed by #657

Comments

@Mathbl
Copy link

Mathbl commented May 24, 2023

Hello,

Would it be possible for licensed status to detect when a record exists for a dependency, but that dependency is not present anymore?

Thanks!

@jonabc
Copy link
Contributor

jonabc commented May 24, 2023

@Mathbl 👋 typically we expect that users will run the cache command after dependencies on a project change to update the stored record files, including removing any outdated record files. Would that solve your need, or am I misunderstanding the request?

@Mathbl
Copy link
Author

Mathbl commented May 24, 2023

Indeed, cache will do the trick! Our use case is that our CI runs licensed status on push and will fail in case a dev forgot to run licensed cache, hence my request.

But, it can also be the dev responsibility to think about running the cache command when adding/removing dependency. You can close the issue if you feel like there is not much value for status to detect dependency removal 🙂.

@jonabc
Copy link
Contributor

jonabc commented May 24, 2023

Ahhh I see what you mean now. From a usage perspective, would you want to see the status command treat these stale files as errors or warnings? Errors are meant to be blocking and will cause the licensed status command to exit with a failing code (1), whereas warnings are meant to be non blocking and will return a success exit code.

@Mathbl
Copy link
Author

Mathbl commented May 24, 2023

Good question! In the case of a missing license/notice, it could cause legal issues, so throwing an error seems correct.

For an extra license/notice from a previously removed dependency, I guess this is less problematic.

If I only take our use case into account, I would like the CI build (running status) to fail, because if it's just a warning and the build succeed, the dev will probably not notice the warning at all.

What do you think?

@jonabc
Copy link
Contributor

jonabc commented May 24, 2023

If I only take our use case into account, I would like the CI build (running status) to fail, because if it's just a warning and the build succeed, the dev will probably not notice the warning at all.

I had the same thought, but I could also see a blocking error being annoying if this isn't a case that someone deeply cares about.

Which means it's a good candidate for a configuration option! I'm going to make the default value be to warn rather than error so that it's not a breaking change from how the tool operates today, and we can consider flipping it so that the default is to raise an error in the next major version bump.

WDYT?

@Mathbl
Copy link
Author

Mathbl commented May 25, 2023

Configuration option seems like a great idea. Thank you 😃

@jonabc
Copy link
Contributor

jonabc commented May 26, 2023

Opened #657 which adds the configuration option to ignore, warn, or error on "stale" metadata files from licensed status.

@jonabc
Copy link
Contributor

jonabc commented Jul 19, 2023

@Japottatweet 👋 you can configure the option in the licensed configuration file. If the configuration option doesn't work please make sure you're using licensed v4.4.0 or greater 🙏

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 a pull request may close this issue.

3 participants
@Mathbl @jonabc and others