Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change error messages start with a lower case letter #445

Merged
merged 1 commit into from
Oct 21, 2020
Merged

Change error messages start with a lower case letter #445

merged 1 commit into from
Oct 21, 2020

Conversation

zhangtbj
Copy link
Contributor

@zhangtbj zhangtbj commented Oct 20, 2020

I am providing the error message list for our service.

I found most of the error messages start with a lower case letter, but few of them forget to correct the format, like:
Updating predicated passed, the annotation was modified.

So I format all related error messages in the controller as same message style:
Change error messages start with a lower case letter

@zhangtbj
Copy link
Contributor Author

Hi all,

Does anyone know how to re-run the Travis test for my PR (I cannot find the restart button in Travis now):
https://travis-ci.org/github/shipwright-io/build/builds/737280956
Thanks!

@SaschaSchwarze0
Copy link
Member

Hi all,

Does anyone know how to re-run the Travis test for my PR (I cannot find the restart button in Travis now):
https://travis-ci.org/github/shipwright-io/build/builds/737280956
Thanks!

I see the restart button (you probably need to login ?), but that won't help unless you fix the test failures like this one:

  Expected

      <string>: "the Bu..."

  to equal       |

      <string>: "The Bu..."

  /home/travis/gopath/src/github.com/shipwright-io/build/pkg/controller/buildrun/buildrun_controller_test.go:651

Also, please enhance the PR description with a rational why you are doing this.

@zhangtbj
Copy link
Contributor Author

Thank you Sascha!

Yes, correct the UT and add description in the comment.

@qu1queee
Copy link
Contributor

@zhangtbj thanks. This PR is good, but I would encourage you to provide a better fix(you can do that as a second PR). What we should do is to trigger a linter for this types of validations for every PR; for this you could do the following:

  • add a new Make target call make staticcheck that runs staticcheck ./...
  • ensure that for every travis pr we execute this target, so probably in travis we will need to install the binary, this can be done via:
$ docker run -it havener/build-environment /bin/bash
root@3f8bdf5e61e6:/# curl -s -LO https://github.com/dominikh/go-tools/releases/download/2020.1.3/staticcheck_linux_amd64.tar.gz
root@3f8bdf5e61e6:/# tar xfz staticcheck_linux_amd64.tar.gz --strip-component 1 -C /tmp  staticcheck/staticcheck && ./tmp/staticcheck -h
Usage of staticcheck:
	staticcheck [flags] # runs on package in current directory
	staticcheck [flags] packages
	staticcheck [flags] directory
	staticcheck [flags] files... # must be a single package
Flags:
  -checks checks
    	Comma-separated list of checks to enable. (default "inherit")
  -debug.cpuprofile file
    	Write CPU profile to file
  -debug.measure-analyzers file
    	Write analysis measurements to file. `file` will be opened for appending if it already exists.
  -debug.memprofile file
    	Write memory profile to file
  -debug.no-compile-errors
    	Don't print compile errors
  -debug.repeat-analyzers num
    	Run analyzers num times
  -debug.unused-graph file
    	Write unused's object graph to file
  -debug.version
    	Print detailed version information about this program
  -explain check
    	Print description of check
  -f format
    	Output format (valid choices are 'stylish', 'text' and 'json') (default "text")
  -fail checks
    	Comma-separated list of checks that can cause a non-zero exit status. (default "all")
  -go version
    	Target Go version in the format '1.x' (default 1.13)
  -show-ignored
    	Don't filter ignored problems
  -tags build tags
    	List of build tags
  -tests
    	Include tests (default true)
  -unused.whole-program
    	Run unused in whole program mode
  -version
    	Print version and exit

@zhangtbj
Copy link
Contributor Author

Sounds good, sure, we can do it in a future PR.
Thanks!

Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 21, 2020
@openshift-merge-robot openshift-merge-robot merged commit 50419d7 into shipwright-io:master Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants