Support nightly #264
Support nightly #264
Conversation
Codecov Report
@@ Coverage Diff @@
## master #264 +/- ##
=======================================
Coverage 42.09% 42.09%
=======================================
Files 38 38
Lines 2960 2960
=======================================
Hits 1246 1246
Misses 1507 1507
Partials 207 207
Continue to review full report at Codecov.
|
| @@ -54,8 +54,7 @@ func (v Version) IsEmpty() bool { | |||
|
|
|||
| // IsNightly returns true if the version is nightly | |||
| func (v Version) IsNightly() bool { | |||
| return string(v) == version.NightlyVersion | |||
|
|
|||
| return strings.Contains(string(v), "nightly") | |||
There was a problem hiding this comment.
This is not true for v1 - although the nightly version should include nightly it doesn't have to.
There was a problem hiding this comment.
seems the only way we can check this is checking weather it contains "nightly" according to #261 @AstroProfundis
There was a problem hiding this comment.
I think if we put nightly in the component spec, then we don't need to check the version to know (from the manifests, whether a version is nightly is external to the version identifier).
There was a problem hiding this comment.
The idea in #261 was that, "nightly" is only a pointer to some version, and we have multiple such versions from daily builds but they are all treated as normal versions, except the one pointed by "nightly".
There was a problem hiding this comment.
A problem is that we must skip nightly versions when getting the latest version of a component, so we need to know which component versions are nightly and which aren't
july2993
force-pushed
the
nightly-versions
branch
from
5674e84
to
594b604
Compare
I think maybe need to instal specified version as v1.2.2-nightly-yyyy-mm-dd, not just "nightly" ? |
Ah, in that case I think we can add a nightly field to the component spec. |
|
@nrc PTAL |
| @@ -188,6 +200,10 @@ func (r *V1Repository) selectVersion(id string, versions map[string]v1manifest.V | |||
| var latest string | |||
| var latestItem v1manifest.VersionItem | |||
| for version, item := range versions { | |||
| if v0manifest.Version(version).IsNightly() { | |||
There was a problem hiding this comment.
Can we inline the 'is nightly' check here so we don't need to use v0manifest.Version?
There was a problem hiding this comment.
I think that we should add to manifests.md that a nightly version must contain the substring nightly. We could also check that in Component.IsValid
There was a problem hiding this comment.
Good catch by the way, I did not think about this issue with nightlies.
| @@ -54,8 +54,7 @@ func (v Version) IsEmpty() bool { | |||
|
|
|||
| // IsNightly returns true if the version is nightly | |||
| func (v Version) IsNightly() bool { | |||
| return string(v) == version.NightlyVersion | |||
|
|
|||
| return strings.Contains(string(v), "nightly") | |||
There was a problem hiding this comment.
A problem is that we must skip nightly versions when getting the latest version of a component, so we need to know which component versions are nightly and which aren't
Co-authored-by: Nick Cameron <nrc@ncameron.org>
cc #261