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.

Sign up for GitHub

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

Jump to bottom

Golic CI #372

Merged
merged 1 commit into from
Mar 18, 2021
Merged

Golic CI #372

merged 1 commit into from
Mar 18, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Mar 17, 2021 •
edited
Loading

  • Makefile: remove license from test target
  • CI, install and run golic v0.4.4
  • license target can modify files, while build step is --dry -x

picture below shows what happens if some file doesn't have expected license
Screenshot 2021-03-17 at 18 16 20

@kuritka
Golic CI
2019ca3 
- Makefile: remove license from test target
- CI, install and run golic v0.4.4
@kuritka kuritka merged commit 8a43732 into master Mar 18, 2021
ytsarev
ytsarev reviewed Mar 18, 2021
View reviewed changes
Makefile
@@ -292,7 +292,7 @@ start-test-app:

# run tests
.PHONY: test
test: license lint
Copy link
Member

Choose a reason for hiding this comment

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

@kuritka why did we remove local invocation?

Copy link
Collaborator Author

@kuritka kuritka Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@ytsarev I supposed you didn't want to check license during a test. Should I amend back ?

Copy link
Member

Choose a reason for hiding this comment

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

@kuritka yep, I meant to amend it with CI, not replace. So CI would catch the missing license in case the PR was created without running local tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ytsarev exactly

@kuritka kuritka mentioned this pull request Mar 18, 2021
Merged
@kuritka kuritka deleted the license-ci branch March 30, 2021 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ytsarev ytsarev ytsarev left review comments

@somaritane somaritane somaritane approved these changes

@k0da k0da k0da approved these changes

@donovanmuller donovanmuller Awaiting requested review from donovanmuller donovanmuller is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kuritka @somaritane @ytsarev @k0da