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

Generation tools improvements #375

Merged
merged 3 commits into from
Mar 19, 2021
Merged

Generation tools improvements #375

merged 3 commits into from
Mar 19, 2021

Conversation

somaritane
Copy link
Contributor

@somaritane somaritane commented Mar 18, 2021

  • boilerplate.go.txt template is restored and handled by golic
  • Local .golic.yaml used to override golic global settings
  • apache2 license is specified explicitly instead of relying on default golic value
  • controller-gen version is aligned with OperatorSDK 1.3.0
  • tools are auto-installed with go install
  • make help is default target
  • go v1.16 is now required for local setup (TODO v1.16 code upgrade will be done in separate PR )

@somaritane somaritane requested a review from k0da March 18, 2021 07:20
Copy link
Collaborator

@kuritka kuritka left a comment

Choose a reason for hiding this comment

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

@somaritane looks great 🦾 , please consider my suggestions

@somaritane somaritane requested a review from kuritka March 18, 2021 07:35
@somaritane somaritane force-pushed the gen-tools-improvements branch from dcd9c9b to 5f45684 Compare March 18, 2021 13:49
@somaritane somaritane force-pushed the gen-tools-improvements branch from 5f45684 to 132e783 Compare March 18, 2021 18:01
@somaritane somaritane requested a review from kuritka March 18, 2021 18:06
Makefile Outdated
license: golic
$(GOLIC) inject -c "2021 Absa Group Limited"
license:
$(call golic,--dry -t apache2)
Copy link
Member

Choose a reason for hiding this comment

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

Why is it dry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iirc it was suggested as precaution by @kuritka, so that make license target only indicates changes but not writes to files.

Copy link
Collaborator

@kuritka kuritka Mar 19, 2021

Choose a reason for hiding this comment

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

@somaritane , license target must modify files (if file is new inject license automatically). Don't know in which context we were talking about. Sorry if there was some misunderstandings caused by me.

Copy link
Member

Choose a reason for hiding this comment

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

So --dry should be removed here, correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So --dry should be removed here, correct?

@ytsarev , yes should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 thanks for clarification @kuritka ! will amend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kuritka @ytsarev amended

@somaritane somaritane mentioned this pull request Mar 19, 2021
@somaritane somaritane requested a review from kuritka March 19, 2021 09:46
Makefile Outdated
license: golic
$(GOLIC) inject -c "2021 Absa Group Limited"
license:
$(call golic,--dry -t apache2)
Copy link
Collaborator

@kuritka kuritka Mar 19, 2021

Choose a reason for hiding this comment

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

@somaritane , license target must modify files (if file is new inject license automatically). Don't know in which context we were talking about. Sorry if there was some misunderstandings caused by me.

@somaritane somaritane force-pushed the gen-tools-improvements branch from dc73a2e to 378e227 Compare March 19, 2021 10:23
Makefile Outdated
license: golic
$(GOLIC) inject -c "2021 Absa Group Limited"
license:
$(call golic,--dry -t apache2)
Copy link
Member

Choose a reason for hiding this comment

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

So --dry should be removed here, correct?

@somaritane somaritane requested review from kuritka and ytsarev March 19, 2021 12:13
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

Please resolve the conflicts

Timofey Ilinykh added 3 commits March 19, 2021 13:17
- boilerplate.go.txt template is restored and handled by golic
- Local .golic.yaml used to override golic global settings
- apache2 license is specified explicitly instead of relying on default golic value
- controller-gen version is aligned with OperatorSDK 1.3.0
- tools are auto-installed with go install
- make help is default target
- go v1.16 is now required for local setup (TODO v1.16 code upgrade will be done in separate PR )
@somaritane somaritane force-pushed the gen-tools-improvements branch from 066f601 to 8d3ad55 Compare March 19, 2021 12:18
@somaritane
Copy link
Contributor Author

Please resolve the conflicts

@ytsarev thanks for pointing, done

Copy link
Collaborator

@kuritka kuritka left a comment

Choose a reason for hiding this comment

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

🦾 GTM

@somaritane somaritane merged commit d0cef5b into master Mar 19, 2021
@somaritane somaritane deleted the gen-tools-improvements branch March 19, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants