Skip to content

Conversation

@dagitses
Copy link
Contributor

@dagitses dagitses commented Dec 5, 2022

@vercel
Copy link

vercel bot commented Dec 5, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
torchci ⬜️ Ignored (Inspect) Dec 21, 2022 at 7:28PM (UTC)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 5, 2022
@dagitses dagitses changed the title add functions to test API compatibility changes remove flexible parameter Dec 5, 2022
@dagitses dagitses changed the title remove flexible parameter add functions to test API compatibility changes Dec 5, 2022
This was referenced Dec 5, 2022
@dagitses dagitses force-pushed the pr1221 branch 5 times, most recently from d74387f to d0cbc71 Compare December 5, 2022 12:45
@dagitses dagitses force-pushed the pr1221 branch 2 times, most recently from 5c3f571 to ea013af Compare December 5, 2022 13:00
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1298).
* #1291
* #1221
* #1304
* #1303
* #1302
* #1301
* #1299
* __->__ #1298

track the line number where a parameter is defined

Summary:
This will allow us to propagate the line number to violations.

Test Plan:
Rely on CI.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1299).
* #1291
* #1221
* #1304
* #1303
* #1302
* #1301
* __->__ #1299
* #1298

use the parameter line number for the moved param check

Test Plan: Verified in test PR.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1301).
* #1291
* #1221
* #1304
* #1303
* #1302
* __->__ #1301
* #1299
* #1298

extract requiredness check to a single function

Summary: This was previously duplicated across two checks.

Test Plan: Rely on CI.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1302).
* #1291
* #1221
* #1304
* #1303
* __->__ #1302

simplify parameter reordering check

Summary:

The previous check was very noisy when a parameter was added. This will
allow us to simplify it down the line.

Test Plan: Rely on CI.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1304).
* #1291
* #1221
* __->__ #1304
* #1303

no longer track position for each parameter

Summary: This can be tracked by its order in the Parameters object.

Test Plan: Rely on CI.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1303).
* #1227
* #1226
* #1225
* #1224
* #1223
* #1221
* #1304
* __->__ #1303

use difflib to better detect changes to positional parameters

Summary:
Fundamentally, for parameters that may be passed positionally, we care
about which from the previous function correspond to the later
function. Diffing is a clean way to do that.

Test Plan: Rely on CI. Verify end-to-end in GitHub UI.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1312).
* #1291
* #1227
* #1226
* #1225
* #1224
* #1223
* #1221
* __->__ #1312

[Reland] no longer track position for each parameter

Summary: This can be tracked by its order in the Parameters object.

Test Plan: Rely on CI.
dagitses added a commit that referenced this pull request Dec 16, 2022
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/pytorch/test-infra/pull/1315).
* #1291
* #1227
* #1226
* #1225
* #1224
* #1223
* #1221
* __->__ #1315

test and fix that adding optional parameters is ok

Test Plan: Rely on CI.
@dagitses dagitses force-pushed the pr1221 branch 5 times, most recently from bb298b9 to b6e864f Compare December 21, 2022 18:55
mikey dagitses added 5 commits December 21, 2022 14:05
Summary: TDD

Test Plan: Verified tests fail.
Summary:
We are able to loosen this check, to support adding forwarding
functions.

Test Plan: Rely on CI.
Summary:
This will be a common recipe when renaming or moving a function to
another file. Leave a breadcrumb to call it.

Test Plan: TDD: verified test is broken
…iadic args

Summary:
The before version having variadic args is meaningless.

Test Plan: Verified broken test is fixed.
@facebook-github-bot
Copy link
Contributor

Hi @dagitses!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants