Skip to content
This repository was archived by the owner on Oct 1, 2023. It is now read-only.

Conversation

samestep
Copy link
Contributor

@samestep samestep commented Apr 6, 2021

For users that do their own preprocessing of linter output, it is more convenient to be able to just produce a JSON array of objects containing the fields that this action requires, rather than redundantly reassembling lines that were already parsed using a regex, only to pass those lines to this action to be re-parsed by the same regex.

  • Add a mode input
  • Use the mode input with parseOutput to correctly handle the 'json' case

I tested this in my personal sandbox repo.

Needs to be merged before pytorch/pytorch#55569.

@samestep samestep marked this pull request as ready for review April 8, 2021 21:27
@samestep samestep requested review from malfet and suo April 8, 2021 21:27
this should be the tip of the pull request, obtainable by running
`git rev-parse ${GITHUB_SHA}^2`"
required: true
mode:
Copy link
Member

Choose a reason for hiding this comment

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

do we even need regex mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not anymore, but I'm keeping it around for now so that PyTorch CI doesn't break while we wait for pytorch/pytorch#55569 to land; we can definitely remove it later

@samestep samestep merged commit 8977c4f into pytorch:master Apr 8, 2021
@samestep samestep deleted the json-mode branch April 8, 2021 21:35
facebook-github-bot pushed a commit to pytorch/pytorch that referenced this pull request Apr 9, 2021
Summary:
This PR

- adds a `tools/translate_annotations.py` script that
  - parses annotations into JSON using the regexes that we were previously passing to [`pytorch/add-annotations-github-action`](https://github.com/pytorch/add-annotations-github-action) and
  - uses `git diff-index` to translate the line numbers for those annotations from the PR `merge` onto the PR `head`, since (as of #54967) we now run CI on the former instead of the latter;
- modifies the `flake8-py3` and `clang-tidy` jobs to use that script and thus upload JSON in their artifacts instead of raw text; and
- modifies the "Add annotations" workflow to specify `mode: json` to allow it to use those preprocessed annotations.

Depends on pytorch/add-annotations-github-action#18.

Pull Request resolved: #55569

Test Plan:
You can run the unit tests with this command:
```
python tools/test/test_translate_annotations.py
```
I also tested the entire system together in my personal sandbox repo.

Reviewed By: malfet

Differential Revision: D27662161

Pulled By: samestep

fbshipit-source-id: ecca51b79b9cf00c90fd89f0d41d0c7b89d69c63
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
Summary:
This PR

- adds a `tools/translate_annotations.py` script that
  - parses annotations into JSON using the regexes that we were previously passing to [`pytorch/add-annotations-github-action`](https://github.com/pytorch/add-annotations-github-action) and
  - uses `git diff-index` to translate the line numbers for those annotations from the PR `merge` onto the PR `head`, since (as of pytorch#54967) we now run CI on the former instead of the latter;
- modifies the `flake8-py3` and `clang-tidy` jobs to use that script and thus upload JSON in their artifacts instead of raw text; and
- modifies the "Add annotations" workflow to specify `mode: json` to allow it to use those preprocessed annotations.

Depends on pytorch/add-annotations-github-action#18.

Pull Request resolved: pytorch#55569

Test Plan:
You can run the unit tests with this command:
```
python tools/test/test_translate_annotations.py
```
I also tested the entire system together in my personal sandbox repo.

Reviewed By: malfet

Differential Revision: D27662161

Pulled By: samestep

fbshipit-source-id: ecca51b79b9cf00c90fd89f0d41d0c7b89d69c63
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants