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

Jump to bottom

Lint rule that would fail for PRs containing subdmodule updates #74326

Open
malfet opened this issue Mar 16, 2022 · 11 comments · May be fixed by #76212
Open

Lint rule that would fail for PRs containing subdmodule updates #74326

malfet opened this issue Mar 16, 2022 · 11 comments · May be fixed by #76212
Labels
good first issue module: bootcamp module: ci triaged
Projects
PyTorch Dev Infra Backlog

Comments

@malfet
Copy link

@malfet malfet commented Mar 16, 2022 •
edited by pytorch-bot bot

🚀 The feature, motivation and pitch

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged.

Alternatives

No response

Additional context

No response

cc @seemethere @malfet @pytorch/pytorch-dev-infra
@malfet malfet added module: bootcamp good first issue module: ci labels Mar 16, 2022
@malfet malfet added this to Needs Triage in PyTorch Dev Infra Backlog via automation Mar 16, 2022
@zou3519 zou3519 added the triaged label Mar 16, 2022
@asmita410
Copy link

@asmita410 asmita410 commented Mar 25, 2022

Hey! I am an absolute beginner, I would like to contribute to this project. Am I allowed to take this up? Also suggest some relevant resources.

@janeyx99
Copy link

@janeyx99 janeyx99 commented Mar 26, 2022

@asmita410 Yes you can take this up and link me for a review with your draft. We anticipate this task will take ~1 week.

Our lints can be found in this GitHub action yaml https://github.com/pytorch/pytorch/blob/master/.github/workflows/lint.yml. Feel free to Google some resources about GitHub actions in general for more context on the syntax.

For how to get access to the PR body, you can look at

pytorch/.github/workflows/_linux-test.yml

Line 82 in 1dab71a

PR_BODY: ${{ github.event.pull_request.body }}
.

For how to get changed files in a PR, you can look at

pytorch/tools/testing/test_selections.py

Line 158 in 0effac2

def _query_changed_test_files() -> List[str]:
for a Python implementation or just a shell command.

@janeyx99 janeyx99 moved this from Needs Triage to Bootcamp in PyTorch Dev Infra Backlog Mar 29, 2022
@nik1097
Copy link

@nik1097 nik1097 commented Apr 6, 2022

Hey @janeyx99 could I work on this issue? If it's not already taken?

k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 9, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
fa97f9c 
…ch#74326
@janeyx99
Copy link

@janeyx99 janeyx99 commented Apr 11, 2022

@nik1097 when do you think you could have a draft PR by? Since @asmita410 hasn't worked on this in the last 2 weeks, I think you can go for it!

@nik1097
Copy link

@nik1097 nik1097 commented Apr 13, 2022

Hey, @janeyx99 thank you for replying. I'm currently working on another issue, and I also see someone else trying to work on it. I will wait for them to complete.

@janeyx99
Copy link

@janeyx99 janeyx99 commented Apr 13, 2022

@nik1097 no worries! have fun with the other issue :) Who do you see working on the issue though? I am unaware of any attempts.

@k-bishop
Copy link

@k-bishop k-bishop commented Apr 14, 2022 •
edited

hey @janeyx99, @nik1097 might be referring to me (I mistakenly opened a PR to the main repo rather than my fork a few days ago). I am sporadically playing around with this, and happy to take it up more seriously if no one has dibs already?

@janeyx99
Copy link

@janeyx99 janeyx99 commented Apr 15, 2022

@k-bishop go for it! do you think you could have a PR ready within 2 weeks?

@k-bishop
Copy link

@k-bishop k-bishop commented Apr 15, 2022

@janeyx99 yes i think so

k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 15, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
624ba60 
…ch#74326
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 21, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
a53a6fb 
…ch#74326
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 21, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
18a8201 
…ch#74326
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 21, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
63aca20 
…ch#74326
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 21, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
22d3638 
…ch#74326

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged. This allows intentional submodule updates to still make it through the linter.
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 21, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
d71dbac 
…ch#74326

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged. This allows intentional submodule updates to still make it through the linter.
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 22, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
1e62ae5 
…ch#74326

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged. This allows intentional submodule updates to still make it through the linter.
@k-bishop k-bishop linked a pull request Apr 22, 2022 that will close this issue
Open
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 22, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
04e3c97 
…ch#74326

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged. This allows intentional submodule updates to still make it through the linter.
k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 22, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
fdea589 
…ch#74326

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged. This allows intentional submodule updates to still make it through the linter.
@k-bishop
Copy link

@k-bishop k-bishop commented Apr 22, 2022 •
edited

@janeyx99 sorry for the commit chaos on this. #76212 should be ready for review now. It's failing one of the linux test builds on gh actions, which doesn't seem to be related to my change. Is there a way I can rerun that one specific test? Any other suggestions?

Edit: looks like its a long-failing test build: https://github.com/pytorch/pytorch/actions?query=workflow%3Apytorch-xla-linux-bionic-py3.7-clang8

k-bishop added a commit to k-bishop/pytorch that referenced this issue Apr 22, 2022
@k-bishop
Lint rule that would fail for PRs containing subdmodule updates pytor…
0bc3bc1 
…ch#74326

When submodules gets updated after checkout, devs often forget to run git submodule update command and propose PR that effectively reverts the update.

Prevent those accidental updates by requiring one to add "submodule-update" keyword to PR description before change can be merged. This allows intentional submodule updates to still make it through the linter.
@janeyx99
Copy link

@janeyx99 janeyx99 commented Apr 25, 2022

@janeyx99 sorry for the commit chaos on this. #76212 should be ready for review now. It's failing one of the linux test builds on gh actions, which doesn't seem to be related to my change. Is there a way I can rerun that one specific test? Any other suggestions?

Taking a look now and will respond on the PR itself :) Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue module: bootcamp module: ci triaged
Projects
PyTorch Dev Infra Backlog
Bootcamp
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Lint rule that would fail for PRs containing subdmodule updates #74326 k-bishop/pytorch
6 participants
@malfet @zou3519 @nik1097 @k-bishop @janeyx99 @asmita410