-
-
Notifications
You must be signed in to change notification settings - Fork 551
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 issue 3155 by running scancode-reindex-licenses
subcommand instead of using --reindex-licenses
flag
#3159
Fix issue 3155 by running scancode-reindex-licenses
subcommand instead of using --reindex-licenses
flag
#3159
Conversation
This script provides a CLI entry point for the `scancode-reindex- licenses` subcommand introduced by 5361052. Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
The `--reindex-licenses` flag has been removed from the main `scancode` executable into the separate subcommand `scancode-reindex-licenses`. Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
As reindexing licenses is an exceptional operation, a separate script is not preferable. Hence, the script has been removed. In the earlier Dockerfile, `./scancode --reindex-licenses` did two jobs: 1. The initial configuration 2. Reindexing of licenses In the new Dockerfile, `./configure` is first run to configure the venv. This is followed by a run of `scancode-reindex-licenses`. Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
Dockerfile best practice is to combine successive RUN commands so as to minimise image layers. Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
The test should ensure that scancode-toolkit is capable of being built successfully using Docker. Additionally, the built script should run successfully. Signed-off-by: Abhishek Kumar <abhi.kr.2100@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok to me @abhi-kr-2100 but we need a project maintainer to review
Thanks for taking the time to review this @s01ipsist. Let me gently ping @pombredanne. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! thank you ++
Merging now
@abhi-kr-2100 Thanks! |
@abhi-kr-2100 I did not pay attention to the details, but your changes have disabled the Azure pipelines entirely. I am going to revert this urgently |
@pombredanne Oops. Please do the needful. |
This was mistakenly disabled with #3159 Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Fixes #3155
This PR only modifies the Dockerfile. In the erroneous Dockerfile,
./scancode --reindex-licenses
was used toSince the
--reindex-licenses
flag has been removed, this led to an error. The fix is to use the new subcommandscancode-reindex-licenses
to do the reindexing. Configuration is done prior to the reindexing using the./configure
script.Tasks
Run tests locally to check for errors.
Signed-off-by: Abhishek Kumar abhi.kr.2100@gmail.com