CFE-4560: Implement Step 3 of cfbs convert#280
Conversation
larsewi
left a comment
larsewi
left a comment
There was a problem hiding this comment.
Great work 🚀 Some smaller comments
| return download_urls, reported_checksums | ||
|
|
||
|
|
||
| def get_all_download_urls(min_version=None): |
There was a problem hiding this comment.
Maybe s/decoupled/refactored/ in commit message?
| @@ -1294,7 +1297,67 @@ def cfbs_convert_git_commit( | |||
| print( | |||
| "The next conversion step is to handle files which have custom modifications." | |||
There was a problem hiding this comment.
This functions has grown to over 260 lines of code. Maybe we should start thinking about refactoring? The different steps could be separated into to different functions
There was a problem hiding this comment.
For a code implementation of a long interactive process like this, I don't mind the function being long.
Perhaps the implementation could be moved to convert.py?
| try: | ||
| cfbs_convert_git_commit(commit_message) | ||
| except: | ||
| log.warning("Git commit failed, continuing without committing...") |
There was a problem hiding this comment.
Maybe it would be nice to have the option to abort here?
…sn't exist Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com>
| download_urls[version] = HARDCODED_URLS[version] | ||
| reported_checksums[version] = HARDCODED_CHECKSUMS[version] |
There was a problem hiding this comment.
Maybe you could even have the URLs and SHAs as a tuple within the object
| download_urls[version] = HARDCODED_URLS[version] | |
| reported_checksums[version] = HARDCODED_CHECKSUMS[version] | |
| download_urls[version], reported_checksums[version] = WHAT_EVER_YOU_WANT_TO CALL_IT[version] |
olehermanse
left a comment
olehermanse
left a comment
There was a problem hiding this comment.
Cool, I might have more feedback after testing it again :)
3 participants
No description provided.