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

fix: ci for go 1.19 and standard generated header #1301

Closed
wants to merge 4 commits into from
Closed

fix: ci for go 1.19 and standard generated header #1301

wants to merge 4 commits into from

Conversation

stevenh
Copy link
Contributor

@stevenh stevenh commented Nov 13, 2022 •
edited
Loading

Summary

Fix continuous integration with go 1.19 and use the standard generated header for golang files.

Changes

  • Use the standard generated header golang files.
  • Fix ci so it works with go 1.19, only checking format and generate for this.
  • Centralise ci in the workflow file.
  • Run ci on the supported versions of golang 1.18 and 1.19.
  • Run go mod tidy on _codegen module.

Motivation

Allows tools which comply with the recommended header to correctly detect the files are generated.

Allow users on go 1.19+ to contribute without having to install old versions of golang.

Centralise the ci in the github workflow file so its easier to maintain.

@stevenh
chore: standard generated header
f494ad0 
Use the standard generated header for golang files as defined by:
https://pkg.go.dev/cmd/go#hdr-Generate_Go_files_by_processing_source
@stevenh stevenh force-pushed the chore/generate-header branch 2 times, most recently from ce509e1 to 55a9a2e Compare November 13, 2022 10:59
@stevenh stevenh changed the title chore: standard generated header fix: ci for go 1.19 and standard generated header Nov 13, 2022
@stevenh stevenh force-pushed the chore/generate-header branch 2 times, most recently from 533b677 to 813a931 Compare November 13, 2022 11:13
@stevenh
ci: run gofmt and go generate conditionally
e93b9f3 
Don't run gofmt or go generate on go versions below 1.19 as it
introduced formatting changes for block comments hence the output is
different from previous versions.

This also centralises the ci in main.yml making it easier to maintain.

Also:
* Add go mod tidy step.
* Bump go versions to the supported versions 1.18 and 1.19
@stevenh stevenh marked this pull request as ready for review November 13, 2022 11:17
@stevenh
ci: include unknown files in checks
05e2dda 
Include unknown files to ensure we capture the case where a new file is
generated but not committed.
@stevenh
ci: no verbose test
64f1c6d 
Don't use verbose mode for tests as it makes it harder to find tests
that fail.
@lisitsky
Copy link
Contributor

Hi, @stevenh !
I've got the same problem with CI for different go versions simultaneously.
Look like code gets generated code in different ways with space characters, so it cannot fit all versions. Or may be checks should be different for different versions.
#1252

@stevenh
Copy link
Contributor Author

stevenh commented Nov 15, 2022

Yer my take as just to go with the latest formatting 1.19+, and skip tests on earlier versions.

@kishaningithub kishaningithub mentioned this pull request Dec 17, 2022
Closed
@dolmen dolmen added the internal/codegen Change related to internal code generation label Jul 7, 2023
@dolmen
Copy link
Collaborator

dolmen commented Jul 7, 2023

Too many unrelated changes in a single MR. This is too much work to review and accept/reject individual changes.

Please resubmit changes in separate MR well contained if they still apply.

@dolmen dolmen closed this Jul 7, 2023
@dolmen dolmen added the github_actions Pull requests that update GitHub Actions code label Jul 7, 2023
@dolmen
Copy link
Collaborator

dolmen commented Jul 7, 2023

About Centralise the ci in the github workflow file so its easier to maintain., I disagree because it make harder to debug the CI build steps on a developer's machine. You should at least explain why you think it's easier to maintain.

@stevenh
Copy link
Contributor Author

stevenh commented Aug 20, 2023 •
edited
Loading

Have split this up into the 2 separate PRs with the two key change sets

For the workflow change if you want to test locally I would recommend act for that. However as the flows themselves are very basic, they are unlikely to need any debugging.

Two clean up steps:

  • go mod tidy - Only needed if dependencies are changed, something developers following best practices are used to running
  • go generate ./... - Only needed if templates are changes, so something people should know to run.

Two validation steps, which are both handled by editors so should be no-op for the developer:

  • gofmt -w .
  • go vet ./...

Hope this helps.

@stevenh stevenh deleted the chore/generate-header branch August 20, 2023 15:45
@stevenh
Copy link
Contributor Author

stevenh commented Aug 20, 2023

Looks like the standard headers have already been done, didn't spot that as this was still pending, so just workflow improvements left.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
github_actions Pull requests that update GitHub Actions code internal/codegen Change related to internal code generation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stevenh @lisitsky @dolmen