-
Notifications
You must be signed in to change notification settings - Fork 13
CFE-3847: Made possible to update modules added by url #226
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
Conversation
bcdd104 to
9b6ee91
Compare
a314117 to
c7a9df6
Compare
7bed220 to
9a2cc97
Compare
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.
Please add shell tests for updating module (added with URL). Also include test cases where:
- steps changed
- input changed
When running cfbs update on the following project
$ cfbs status
Name: Example project
Description: Example description
File: cfbs.json
Modules:
001 masterfiles @ 3.24.2 / 5c49f1ed1c3a4ecfc46b4b61296ff5dd38991f33 (Downloaded)
002 compliance-report-imports @ / 1a08bf82757326039e1c1836f8e9ff064f4a9e44 (Downloaded)
It says that masterfiles is already up-to-date. But it does not mention compliance-report-imports (added with URL).
$ cfbs update
Module 'masterfiles' already up to date
Modules are already up to date
1eeaee0 to
8736e7b
Compare
|
Looks good, but please add some shell tests. You can for example add this module using URL, modify the commit SHA, input, build steps and run |
49e7aff to
dec5050
Compare
larsewi
left a comment
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.
Great work 🚀
olehermanse
left a comment
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.
Please rebase on latest master, rename your test with a new number (39 is no longer the next number since other tests have been added) and add your test to all.sh
dec5050 to
ada3713
Compare
7fbd884 to
dec5050
Compare
olehermanse
left a comment
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.
It looks like you made a mistake and went back in time here - Your shell test needs to be renamed (again) and added to all.sh.
This is why you should use |
The part of the loop in update_command responsible for actually updating modules was moved to its own function. It is done by passing a module updates class that tracks the updates generally. Signed-off-by: Victor Moene <victor.moene@northern.tech>
dec5050 to
3f1eee9
Compare
c549d86 to
b03caa8
Compare
Ticket: CFE-3847 Signed-off-by: Victor Moene <victor.moene@northern.tech>
The url field makes the update command clone the repo. However, the github profile and repo link in the build examples are placeholders, and make the test fail. Signed-off-by: Victor Moene <victor.moene@northern.tech>
b03caa8 to
5e8bc60
Compare
Signed-off-by: Victor Moene <victor.moene@northern.tech>
Signed-off-by: Victor Moene <victor.moene@northern.tech>
5e8bc60 to
8698f13
Compare
Added a way to update modules added by url. I tried to do it with minimal extra code inside the update loop. I am not sure if we want to delete the older
commit/repo