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

Add check for BQ compatible schema-changes to PR review checklist #316

Open
fbertsch opened this issue Apr 22, 2019 · 4 comments
Open

Add check for BQ compatible schema-changes to PR review checklist #316

fbertsch opened this issue Apr 22, 2019 · 4 comments

Comments

@fbertsch
Copy link
Contributor

This comes from the GCP schema handling doc.

We will need to initially start minimizing schema-incompatible changes for BQ. This only means that we can't change field types and can't remove fields. The former has historically (AFAIK) not been a common change, and the latter can be easily avoided by simply keeping the fields.

@jklukas
Copy link
Contributor

jklukas commented Apr 23, 2019

Note on implementation: this test will likely need to pull the pre-PR state of any changed schemas via some git commands, then do some amount of parsing of the two versions of the schema to detect the conditions we want to avoid.

@fbertsch
Copy link
Contributor Author

Those are good points @jklukas! I meant for this to just be added to the PR review checklist. Mark and I were discussing a longer-term CI test for schema changes, with a short-term improvement being akin to the one you mentioned, should we move that to a separate issue?

@jklukas
Copy link
Contributor

jklukas commented Apr 23, 2019

should we move that to a separate issue

Indeed. I interpreted this as being about CI tests. Apologies for missing the scope. I agree that adding to a checklist is a good first step that can be done immediately.

@fbertsch fbertsch changed the title Add check for BQ compatible schema-changes to PR review Add check for BQ compatible schema-changes to PR review checklist Apr 23, 2019
@fbertsch
Copy link
Contributor Author

Okay great. Updated the title of this issue to make it more clear, and let's move CI discussion to #317.

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

No branches or pull requests

2 participants