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

chore: dont format auto-generated files #2750

Closed

Conversation

kevgo
Copy link
Contributor

@kevgo kevgo commented Sep 23, 2022

Goimports doesn't provide a built-in mechanism to ignore certain directories. See golang/go#42965. This creates a workaround that is still fast enough.

Related issue(s)

https://github.com/ory-corp/general/issues/735

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security. vulnerability, I
    confirm that I got green light (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #2750 (5a751df) into master (04111f8) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2750      +/-   ##
==========================================
+ Coverage   75.15%   75.16%   +0.01%     
==========================================
  Files         294      294              
  Lines       17068    17068              
==========================================
+ Hits        12828    12830       +2     
+ Misses       3264     3263       -1     
+ Partials      976      975       -1     
Impacted Files Coverage Δ
persistence/sql/persister_courier.go 84.84% <0.00%> (+2.02%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@zepatrik zepatrik left a comment

Choose a reason for hiding this comment

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

Why do we have to ignore those?

@kevgo
Copy link
Contributor Author

kevgo commented Sep 23, 2022

@zepatrik This PR is just a backup that allows the Kratos team to deal with build breakage around formatting like https://github.com/ory/kratos/actions/runs/3109025727/jobs/5038851404 without reverting format checking on CI, as was done in #2737.

As we tighten up CI, more such breakage might happen. The root cause is automation like https://github.com/ory/ci/blob/master/sdk/generate/action.yml that generates unformatted code and pushes it directly into the master branch of various repos bypassing CI.

In my opinion, the correct solution is to fix this automation by having it run the things that CI does before it bypasses it. ory/ci#109 is an initial attempt at that.

If you are up for it, we can work together to improve this automation. I'm happy doing most of the work. I need your help getting these PRs shipped and helping teams encountering such build breakage understand the situation and come up with the right fixes while I'm not online.

@zepatrik
Copy link
Member

Makes sense, I'd rather fix the root cause (so the unformatted commit). This will be obsolete then?

@kevgo
Copy link
Contributor Author

kevgo commented Sep 23, 2022

Yes. I'll close these backup-PRs.

@kevgo kevgo closed this Sep 23, 2022
@kevgo kevgo deleted the kg-dont-format-autogenerated-files branch September 23, 2022 15:28
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.

2 participants