Skip to content

Conversation

@bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Nov 3, 2022

In AUTH_CHECKS, "mentionable_users" was "metionable_users".
This pull request fixes this spelling error.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Nov 3, 2022

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@bkmgit bkmgit changed the title Fix spelling error, metionable -> mentionable [ci][tvmbot] Fix spelling error, metionable -> mentionable Nov 3, 2022
Mousius
Mousius previously requested changes Nov 3, 2022
Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

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

There's a line below in this file:

    auth = [AUTH_CHECKS["metionable_users"]]

Can you update there as well and add a test as this almost got broken 😿

@driazati ^^^

@driazati
Copy link
Member

driazati commented Nov 3, 2022

also fyi @Mousius until #12941 merges these files aren't actually used in CI, their main counterparts are (which is why CI passed even though it should have failed on the class definition of Rerun)

@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 4, 2022

Thanks for catching that. Thinking about appropriate tests.

@driazati driazati closed this Nov 15, 2022
@driazati driazati reopened this Nov 15, 2022
@driazati
Copy link
Member

driazati commented Nov 15, 2022

Hey @bkmgit, we recently merged a PR #13276 #13368 that makes it so you can iterate and test on this PR without needing a committer in the loop, can you rebase this PR to fix the conflicts then we can get it merged? thanks!

@driazati driazati dismissed Mousius’s stale review November 15, 2022 20:26

changes are now tested due to #13276, no new tests are necessary

In AUTH_CHECKS, "mentionable_users" was "metionable_users".
This pull request fixes this spelling error.
As pointed out in review, `metionable` is also used on line
674 of github_tvmbot.py, update this to `mentionable`.
@bkmgit
Copy link
Contributor Author

bkmgit commented Nov 16, 2022

@driazati @Mousius Hopefully ok now.

@Mousius
Copy link
Member

Mousius commented Nov 16, 2022

Hey @bkmgit, we recently merged a PR #13276 that makes it so you can iterate and test on this PR without needing a committer in the loop, can you rebase this PR to fix the conflicts then we can get it merged? thanks!

I think you've linked back to this PR @driazati, which one did you mean to refer to?

Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

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

LGTM! Leaving it with you @driazati 😸

@driazati
Copy link
Member

Ah thanks for catching, I meant to link #13368

@driazati driazati merged commit 52739ef into apache:main Nov 16, 2022
@bkmgit bkmgit deleted the patch-3 branch November 17, 2022 18:28
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
)

In AUTH_CHECKS, "mentionable_users" was "metionable_users".
This pull request fixes this spelling error.
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.

4 participants