Skip to content

Commit

Permalink
Improve the error message when a released schema is changed
Browse files Browse the repository at this point in the history
Fixes #4168

Signed-off-by: David Gageot <[email protected]>
  • Loading branch information
dgageot committed Jun 19, 2020
1 parent 3cba8f9 commit 33da370
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 18 deletions.
4 changes: 0 additions & 4 deletions hack/check-schema-changes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,4 @@
# If the change is latest and it is released, we fail the PR for the same reason.
# If the change is in latest and it is not released yet, it is fine to make changes.



set +x

go run hack/versions/cmd/schema_check/check.go
4 changes: 2 additions & 2 deletions hack/versions/cmd/schema_check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ limitations under the License.
package main

import (
"github.com/sirupsen/logrus"
"os"

"github.com/GoogleContainerTools/skaffold/hack/versions/pkg/schema"
)

func main() {
if err := schema.RunSchemaCheckOnChangedFiles(); err != nil {
logrus.Fatal(err)
os.Exit(1)
}
}
29 changes: 18 additions & 11 deletions hack/versions/pkg/schema/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ import (
"errors"
"fmt"
"io/ioutil"
"os"
"path"
"strings"

"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/hack/versions/pkg/diff"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
)

const baseRef = "origin/master"
Expand Down Expand Up @@ -90,23 +88,32 @@ func RunSchemaCheckOnChangedFiles() error {
continue
}

logrus.Infof("structural changes in latest config, checking on Github if latest is released...")
logrus.Warnf("Detected changes to the latest config. Checking on Github if it's released...")
latestVersion, isReleased := GetLatestVersion()
if !isReleased {
color.Green.Fprintf(os.Stdout, "%s is unreleased, it is safe to change it.\n", latestVersion)
logrus.Infof("Schema %q is not yet released. Changes are ok.", latestVersion)
continue
}
color.Red.Fprintf(os.Stdout, "%s is released, it should NOT be changed!\n", latestVersion)

logrus.Errorf("Schema %q is already released. Changing it is not allowed.", latestVersion)

fmt.Printf("\nWhat should I do?\n-----------------\n")
fmt.Printf(" + Please first create a new PR, with a new version, using 'hack/new_version.sh' script.\n")
fmt.Printf(" + If you see this error and it seems incorrect, maybe you have an out-of-date %q local branch.\n", baseRef)
fmt.Printf(" + If this retroactive change is required and harmless to the users, you'll need Admin rights to merge this PR.\n")

filesInError = append(filesInError, configFile)
}

for _, file := range filesInError {
logrus.Errorf(changeDetected(file))
changes, err := git.diffWithBaseline(file)
if err != nil {
logrus.Errorf("failed to get diff: %s", err)
if len(filesInError) > 0 {
fmt.Printf("\nInvalid changes:\n----------------\n")
for _, file := range filesInError {
changes, err := git.diffWithBaseline(file)
if err != nil {
logrus.Errorf("failed to get diff: %s", err)
}
fmt.Print(string(changes))
}
fmt.Print(string(changes))
}

if len(filesInError) > 0 {
Expand Down
2 changes: 1 addition & 1 deletion hack/versions/pkg/schema/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (

func GetLatestVersion() (string, bool) {
current := strings.TrimPrefix(latest.Version, "skaffold/")
logrus.Infof("Current Skaffold version: %s", current)
logrus.Debugf("Current Skaffold version: %s", current)

config, err := ioutil.ReadFile("pkg/skaffold/schema/latest/config.go")
if err != nil {
Expand Down

0 comments on commit 33da370

Please sign in to comment.