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

Fix hashes correctly #17223

Merged

Conversation

vojtechjelinek
Copy link
Member

Overview

  1. This PR fixes or fixes part of #N/A
  2. This PR does the following: Change hashes in the develop_commit_notification.yml to point to the correct version.

Essential Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The linter/Karma presubmit checks have passed locally on your machine.
  • "Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
    • This lets reviewers restart your CircleCI tests for you.
  • The PR is made from a branch that's not called "develop".

Proof that changes are correct

Not possible to provide as the changes first need to be merged to see if they work.

PR Pointers

  • Make sure to follow the instructions for making a code change.
  • If you need a review or an answer to a question, and don't have permissions to assign people, leave a comment like the following: "{{Question/comment}} @{{reviewer_username}} PTAL". Oppiabot will help assign that person for you.
  • For what code owners will expect, see the Code Owner's wiki page.
  • Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
  • Never force push. If you do, your PR will be closed.
  • Some of the e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.

@oppiabot
Copy link

oppiabot bot commented Feb 1, 2023

Hi @vojtechjelinek please assign the required reviewer(s) for this PR. Thanks!

@oppiabot
Copy link

oppiabot bot commented Feb 1, 2023

Hi @vojtechjelinek, can you complete the following:

  1. The proof that changes are correct has not been provided, please make sure to upload a image/video showing that the changes are correct. Or include a sentence saying "No proof of changes needed because" and the reason why proof of changes cannot be provided.
    Thanks!

@gp201
Copy link
Member

gp201 commented Feb 1, 2023

@vojtechjelinek is there any way to test the hashes on a separate repository?

@gp201 gp201 assigned vojtechjelinek and unassigned gp201 Feb 1, 2023
@vojtechjelinek
Copy link
Member Author

@gp201 I've verified it at least locally now.

@oppiabot oppiabot bot added the PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. label Feb 4, 2023
@oppiabot
Copy link

oppiabot bot commented Feb 4, 2023

Hi @vojtechjelinek, the build of this PR is stale and this could result in tests failing in develop. Please update this pull request with the latest changes from develop. Thanks!

Copy link
Member

@gp201 gp201 left a comment

Choose a reason for hiding this comment

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

LGTM

@oppiabot oppiabot bot unassigned gp201 Feb 5, 2023
@oppiabot
Copy link

oppiabot bot commented Feb 5, 2023

Unassigning @gp201 since they have already approved the PR.

@oppiabot oppiabot bot added the PR: LGTM label Feb 5, 2023
@oppiabot
Copy link

oppiabot bot commented Feb 5, 2023

Hi @vojtechjelinek, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: don't merge - STALE BUILD The build on this PR is stale and should be restarted. PR: LGTM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants