Skip to content

Disable auto code generator for postgres migrations#3820

Merged
c-du merged 3 commits intomasterfrom
cong/no-auto
Nov 16, 2022
Merged

Disable auto code generator for postgres migrations#3820
c-du merged 3 commits intomasterfrom
cong/no-auto

Conversation

@c-du
Copy link
Copy Markdown
Contributor

@c-du c-du commented Nov 15, 2022

Description

Disable auto code generator for postgres migrations.
This one will be after the schema freeze PR and for 3.74 #3769

Checklist

  • Investigated and inspected CI test results~
  • [ ] Unit test and regression tests added
  • [ ] Evaluated and added CHANGELOG entry if required
  • ~[ ] Determined and documented upgrade steps`
  • [ ] Documented user facing changes (create PR based on [openshift/openshift-docs](https://github.com/openshift/openshift-docs) and merge into rhacs-docs)~

If any of these don't apply, please comment below.

Testing Performed

TODO(replace-me)
Use this space to explain how you tested your PR, or, if you didn't test it, why
you did not do so. Valid reasons include, for example, "CI is sufficient",
"No testable changes". Feel free to attach JSON snippets, curl commands,
screenshots.

In addition to reviewing your code, reviewers must also review your testing
instructions and make sure they are sufficient.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Nov 15, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@c-du c-du added the do-not-merge A change which is not meant to be merged label Nov 15, 2022
@c-du c-du marked this pull request as ready for review November 15, 2022 18:09
@c-du c-du requested a review from a team as a code owner November 15, 2022 18:09
@c-du c-du requested a review from dashrews78 November 15, 2022 18:09
md2119
md2119 reviewed Nov 15, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@md2119 md2119 left a comment

Choose a reason for hiding this comment

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

Can we add a check in the generator code to errors out if a migration flag is encountered? It will ensure that nobody mistakenly adds a migration.

@ghost
Copy link
Copy Markdown

ghost commented Nov 15, 2022 •
edited by ghost
Loading

Images are ready for the commit at 0e6b70d.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.73.x-12-g0e6b70d786.

@dashrews78
Copy link
Copy Markdown
Contributor

dashrews78 commented Nov 15, 2022

Do we need to update the comment in the generated files or is that OK to leave?
// Code generated by pg-bindings generator. DO NOT EDIT.

Or do we just deal with that if we need to update something in one of those files?

@c-du
Copy link
Copy Markdown
Contributor Author

c-du commented Nov 15, 2022

Let's leave it as is. Most likely and ideally we don't need to update them anymore. They would be obsoleted in a few releases.

@c-du
Copy link
Copy Markdown
Contributor Author

c-du commented Nov 15, 2022

Can we add a check in the generator code to errors out if a migration flag is encountered? It will ensure that nobody mistakenly adds a migration.
In that case, it would be better to remove the migration flags in the generator.

@c-du c-du requested a review from md2119 November 15, 2022 23:30
@c-du c-du enabled auto-merge (squash) November 16, 2022 22:04
@c-du c-du merged commit ce14d08 into master Nov 16, 2022
@c-du c-du deleted the cong/no-auto branch November 16, 2022 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dashrews78 dashrews78 dashrews78 approved these changes

+1 more reviewer

@md2119 md2119 md2119 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/central do-not-merge A change which is not meant to be merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@c-du @dashrews78 @md2119