Version requirements #357
Merged
Version requirements #357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davidcheung
force-pushed
the
version-requirements
branch
from
5ce770c
to
725a6b2
Compare
bmonkman
reviewed
Mar 24, 2021
| @@ -1,7 +1,7 @@ | |||
| VERSION = 0.0.1 | |||
| BUILD ?=$(shell git rev-parse --short HEAD) | |||
| PKG ?=github.com/commitdev/zero | |||
| BUILD_ARGS=-v -trimpath -ldflags=all="-X ${PKG}/cmd.appVersion=${VERSION} -X ${PKG}/cmd.appBuild=${BUILD}" | |||
| BUILD_ARGS=-v -trimpath -ldflags=all="-X ${PKG}/version.AppVersion=${VERSION} -X ${PKG}/version.AppBuild=${BUILD}" | |||
There was a problem hiding this comment.
For real releases this actually gets set by .goreleaser.yml, not the Makefile.
bmonkman
reviewed
Mar 24, 2021
docs/module-definition.md
Outdated
| | `author` | string | Author of the module | | ||
| | `icon` | string | Path to logo image | | ||
| | `parameters` | list(Parameter) | Parameters to prompt users | | ||
| | `zeroVersion` | string(go-semver)| Zero versions its compatible with | |
There was a problem hiding this comment.
Maybe include a link to the go-version readme?
bmonkman
reviewed
Mar 24, 2021
| @@ -107,6 +138,10 @@ func LoadModuleConfig(filePath string) (ModuleConfig, error) { | |||
| log.Fatal("") | |||
| } | |||
|
|
|||
| if !ValidateZeroVersion(config) { | |||
| constraint := config.ZeroVersion.Constraints.String() | |||
| return config, errors.New(fmt.Sprintf("Module's zero requirement not satisfied: expected %s received: %s", constraint, version.AppVersion)) | |||
There was a problem hiding this comment.
Let's add some more details, we are telling them a bit about what's wrong but not how to fix it, and it will be the user who is faced with this message.
bmonkman
reviewed
Mar 24, 2021
| return err | ||
| } | ||
|
|
||
| constraints, constErr := goVerson.NewConstraint(versionString) |
There was a problem hiding this comment.
If the module.yml doesn't have a version specified we should probably just skip the check.
bmonkman
approved these changes
Mar 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
allow modules to declare versions of Zero it would work with
this allows us to introduce breaking change and let users know the modules require newest features of Zero