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: add cypress required checks for branch protection #12970

Merged
merged 3 commits into from
Feb 9, 2021

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Feb 5, 2021

SUMMARY

Continuation of #12921, #12928 and #12969 , this PR adds more required github checks.

TEST PLAN

verify that the correct tests are required after this is merged. If this yml file doesn't match correctly, we will need to rename the checks so that they match or open an infra ticket to Apache to update it.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@eschutho eschutho marked this pull request as draft February 5, 2021 19:10
@eschutho
Copy link
Member Author

eschutho commented Feb 5, 2021

@ktmud I'm queuing up the remaining checks to rebase when the other PRs are merged and then put up for review.

@eschutho eschutho changed the title add cypress required checks chore: add cypress required checks for branch protection Feb 5, 2021
@eschutho eschutho force-pushed the elizabeth/add-cypress-checks branch from e8b0b18 to e614d30 Compare February 8, 2021 21:40
@eschutho eschutho marked this pull request as ready for review February 8, 2021 21:41
@eschutho
Copy link
Member Author

eschutho commented Feb 8, 2021

@ktmud this is the last batch of the previously required checks. Is there anything else we want to add? I noticed there are some 3.8 matrix db tests for example.

@ktmud
Copy link
Member

ktmud commented Feb 8, 2021

Yeah, I think it makes sense to add 3.8 checks since we are going to wait for them anyway.

@eschutho eschutho force-pushed the elizabeth/add-cypress-checks branch from e614d30 to 45f57ac Compare February 8, 2021 21:45
@eschutho
Copy link
Member Author

eschutho commented Feb 8, 2021

Ok, only postgres of the currently required has a 3.8 matrix.. unless we want to add in presto and hive?

@ktmud
Copy link
Member

ktmud commented Feb 8, 2021

I think it makes sense to keep Hive and Presto optional as they are more difficult to set up. If it somehow starts breaking in master, everyone would have to wait for someone with the environment set up to fix it (although docker-compose has made this easy).

I can also see why it's important to make sure master is stable for Presto/Hive, so I'm OK with either way.

@eschutho
Copy link
Member Author

eschutho commented Feb 8, 2021

🏷 ready-to-merge

@superset-github-bot superset-github-bot bot added the need:merge The PR is ready to be merged label Feb 8, 2021
@eschutho
Copy link
Member Author

eschutho commented Feb 8, 2021

@ktmud I saw that we still have required_signatures: true that you requested to be added in the last commit. Is the repo currently set up with that requirement? I can add it in here if you think it's safe.

@nytai
Copy link
Member

nytai commented Feb 8, 2021

@eschutho we should add that option. It doesn't really matter since github will add the signatures anyway if merging is happening via the UI (currently that's the only way to merge), but it will likely appease some of the apache infra 👀

@ktmud ktmud merged commit 9cd0165 into apache:master Feb 9, 2021
@eschutho eschutho deleted the elizabeth/add-cypress-checks branch February 9, 2021 16:57
amitmiran137 pushed a commit to simcha90/incubator-superset that referenced this pull request Feb 10, 2021
* master:
  fix: UI toast typo (apache#13026)
  feat(db engines): add support for Opendistro Elasticsearch (AWS ES) (apache#12602)
  fix(build): black failing on master, add to required checks (apache#13039)
  fix: time filter db migration optimization (apache#13015)
  fix: untranslated text content of Dashboard page (apache#13024)
  fix(ci): remove signature requirements for commits to master (apache#13034)
  fix: add alerts and report to default config (apache#12999)
  docs(changelog): add entries for 1.0.1 (apache#12981)
  ci: skip cypress if no code changes (apache#12982)
  chore: add cypress required checks for branch protection (apache#12970)
  Refresh dashboard list after bulk delete (apache#12945)
  Updates storybook to version 6.1.17 (apache#13014)
  feat: Save datapanel state in local storage (apache#12996)
  fix: added text and changed margins (apache#12923)
  chore: Swap Slack Url 2 more places (apache#13004)
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels need:merge The PR is ready to be merged preset-io size/XS 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants