Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Rename aktualizr-repo to uptane-generator. #1279

Merged
merged 1 commit into from
Aug 14, 2019

Conversation

pattivacek
Copy link
Collaborator

Is this too wordy? I'm open to suggestions or abbreviations, but I want to move away from "aktualizr-repo", which is way to ambiguous and confusing.

@codecov-io
Copy link

codecov-io commented Aug 2, 2019

Codecov Report

Merging #1279 into master will decrease coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1279      +/-   ##
==========================================
- Coverage   79.29%   79.17%   -0.13%     
==========================================
  Files         176      175       -1     
  Lines       10394    10367      -27     
==========================================
- Hits         8242     8208      -34     
- Misses       2152     2159       +7
Impacted Files Coverage Δ
src/aktualizr_uptane_generator/image_repo.cc 92.98% <ø> (ø)
src/aktualizr_uptane_generator/uptane_repo.cc 96.55% <ø> (ø)
src/aktualizr_uptane_generator/director_repo.cc 91.54% <ø> (ø)
src/aktualizr_uptane_generator/image_repo.h 100% <ø> (ø)
src/aktualizr_uptane_generator/repo.cc 96.44% <ø> (ø)
src/aktualizr_uptane_generator/director_repo.h 100% <ø> (ø)
src/aktualizr_uptane_generator/repo.h 83.33% <ø> (ø)
src/aktualizr_uptane_generator/uptane_repo.h 100% <ø> (ø)
src/aktualizr_uptane_generator/main.cc 82.45% <100%> (ø)
src/aktualizr_lite/main.cc 70.55% <0%> (-6.02%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b0768d...a5897e9. Read the comment docs.

eu-siemann
eu-siemann previously approved these changes Aug 2, 2019
Copy link
Contributor

@eu-siemann eu-siemann left a comment

Choose a reason for hiding this comment

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

I'm unable to come up with anything better and happy to get rid of the current confusing name

@lbonn
Copy link
Contributor

lbonn commented Aug 2, 2019

There are some variables and function names with remnants of the former tool name: akrepo, akt_repo.

@Zee314159
Copy link
Contributor

Is this too wordy? I'm open to suggestions or abbreviations, but I want to move away from "aktualizr-repo", which is way to ambiguous and confusing.

aktualizr-uptane-gen or aktualizr-xxx-gen ?

@pattivacek pattivacek force-pushed the refactor/rename-aktualizr-repo branch from 09b8868 to 892ee34 Compare August 2, 2019 16:41
@xcheng-here
Copy link
Contributor

I think it's clear but a bit wordy. Good for customer to understand what it is but maybe a little bit long to us to say it in discussing. Maybe it could have an abbreviation at same time ?

@pattivacek
Copy link
Collaborator Author

aktualizr-uptane-gen or aktualizr-xxx-gen ?

aktualizr-gen-uptane? Or is even just aktualizr-generator good enough? I'm open to discussion and will rebase/redo once we reach a final conclusion.

@lbonn
Copy link
Contributor

lbonn commented Aug 8, 2019

If we want a shorter name, we can also drop the "aktualizr" part: "uptane-generator", "make-uptane"...

@Zee314159
Copy link
Contributor

Aktualizr-meta-gen?

@pattivacek
Copy link
Collaborator Author

If we want a shorter name, we can also drop the "aktualizr" part

I thought we had decided that all aktualizr tools should be prefixed by aktualizr to prevent name collisions and make it clear that these are related, adjacent tools.

@lbonn
Copy link
Contributor

lbonn commented Aug 8, 2019

I thought we had decided that all aktualizr tools should be prefixed by aktualizr to prevent name collisions and make it clear that these are related, adjacent tools.

Ah I was not aware of this specific discussion. But then the question is how "related" and "adjacent" the tool is. It's clear for aktualizr-info which really complements it but in this case it's mostly not aktualizr-specific (in theory at least), we just happen to make use of it for our aktualizr test suite.

@pattivacek
Copy link
Collaborator Author

But then the question is how "related" and "adjacent" the tool is. It's clear for aktualizr-info which really complements it but in this case it's mostly not aktualizr-specific (in theory at least), we just happen to make use of it for our aktualizr test suite.

That's a good point. The only things that really need the prefix are probably bundled tools that are useless without aktualizr. aktualizr-repo is intended for use with aktualizr, but it's really specific to it at all, nor is it bundled for release in the deb packages, nor is it packaged for distribution via Yocto.

Hence, perhaps "uptane-generator" is good enough as it is. Any objections?

@mike-sul
Copy link
Collaborator

what about uptane-repo-gen ? :)

@pattivacek pattivacek force-pushed the refactor/rename-aktualizr-repo branch from 892ee34 to a5897e9 Compare August 13, 2019 10:11
@pattivacek
Copy link
Collaborator Author

Updated to "uptane-generator" after today's conversation. Now's your chance to complain.

This is hopefully somewhat more clear, and since it isn't specific to
aktualizr, nor bundled with aktualizr, "aktualizr" doesn't need to be
part of the name.

Signed-off-by: Patrick Vacek <[email protected]>
@pattivacek pattivacek force-pushed the refactor/rename-aktualizr-repo branch from a5897e9 to 32c73ef Compare August 13, 2019 12:22
@pattivacek
Copy link
Collaborator Author

I cannot get that first Travis test to pass for this. It keeps timing out at 50 minutes. Should we merge anyway, or is there something else to try here?

@lbonn
Copy link
Contributor

lbonn commented Aug 14, 2019

The problem is that I've been cheating with ccache to make it fast enough recently. Since this PR touches a lot of files, it must have invalidated a lot of cache, so we're back to the old running time + overhead of downloading cache.

But whatever, let's not worry about travis for merging.

@pattivacek
Copy link
Collaborator Author

The problem is that I've been cheating with ccache to make it fast enough recently. Since this PR touches a lot of files, it must have invalidated a lot of cache, so we're back to the old running time + overhead of downloading cache.

Agreed. But I'm confused on one point: does the ccache not get updated if the Travis build doesn't complete? I'm not surprised the first attempt failed, but I was hoping the incomplete build would've made the next one faster, and that doesn't seem to be the case.

Also, #1276 is failing for the same reason despite not changing much.

But whatever, let's not worry about travis for merging.

Agreed, at least for this case.

@pattivacek pattivacek changed the title Rename aktualizr-repo to aktualizr-uptane-generator. Rename aktualizr-repo to uptane-generator. Aug 14, 2019
@lbonn
Copy link
Contributor

lbonn commented Aug 14, 2019

Agreed. But I'm confused on one point: does the ccache not get updated if the Travis build doesn't complete? I'm not surprised the first attempt failed, but I was hoping the incomplete build would've made the next one faster, and that doesn't seem to be the case.

Unfortunately no, if the build timeouts nothing is uploaded. There is a trick we could use that was mentioned in a blog post: set a timeout in the test itself so that it fails but never timeouts from the point of view of Travis. We might resort to that at some point.

@pattivacek pattivacek merged commit 50cf437 into master Aug 14, 2019
@pattivacek pattivacek deleted the refactor/rename-aktualizr-repo branch August 14, 2019 09:10
@pattivacek
Copy link
Collaborator Author

Unfortunately no, if the build timeouts nothing is uploaded.

Ah, I see. So that means if a build is timing out, we'll get stuck in a cycle like the current one where we have to wait until we just get lucky and it succeeds. Could we at least update the ccache after compilation but before running the tests?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants