Skip to content
This repository has been archived by the owner on Nov 14, 2022. It is now read-only.

Adds YAPF to requirements and as a CI step #1075

Merged
merged 4 commits into from
Feb 8, 2021
Merged

Adds YAPF to requirements and as a CI step #1075

merged 4 commits into from
Feb 8, 2021

Conversation

dtasev
Copy link
Contributor

@dtasev dtasev commented Feb 5, 2021

Summary of work

Adds YAPF to requirements and as a CI step

How to test your work

Tests should pass

Fixes #915

@keiranjprice101
Copy link
Contributor

keiranjprice101 commented Feb 5, 2021

Possibly wishful thinking, but is it possible to add an action that will run yapf and commit the output ?

edit: it appears something like this is possible https://github.com/diegovalenzuelaiturra/yapf-action

@dtasev
Copy link
Contributor Author

dtasev commented Feb 5, 2021

I don't mind that tbh - do you think it might be annoying if you decide to add more changes but then you can't push & have to rebase/merge the formatting changes?

@keiranjprice101
Copy link
Contributor

You can just pull the formatting change and continue from there no?

@dtasev
Copy link
Contributor Author

dtasev commented Feb 5, 2021

Yeah but if you don't notice and continue working after pushing, then at some point you might get a lot of conflicts. Perhaps some bot would be better, i.e. if you comment @Format or something it just does it and commits?

@keiranjprice101
Copy link
Contributor

keiranjprice101 commented Feb 5, 2021

That could be nice, especially if we can combine it with a branch protection rule that wont allow merging until the @Format has been triggered (Or whatever the manual trigger event is)

@rickybassom
Copy link
Contributor

You can have a look at this code I used to trigger code scanning by commenting on a PR. In the end I decided to go for a workflow_dispatch event that can be triggered from Actions > Code Scanning > Run Workflow.

I suppose both methods would work for triggering a YAPF action.

@dtasev
Copy link
Contributor Author

dtasev commented Feb 8, 2021

Hmm I did some looking around and https://unibeautify.com/ looks like exactly what we were discussing above. I will try to integrate it in to this repo.

@dtasev dtasev force-pushed the 915_enforce_yapf branch 6 times, most recently from 0bd5602 to 322d825 Compare February 8, 2021 09:53
@dtasev
Copy link
Contributor Author

dtasev commented Feb 8, 2021

Unibeautify should provide that ability to merge the formatted changes, however it doesn't seem to be working right now and I'm trying to get some help from the CI maintainers at Unibeautify/ci-issues#5

In the meantime I've added back the YAPF CI job and I don't think this should hold up this PR

@dtasev dtasev merged commit a64efb6 into master Feb 8, 2021
@dtasev dtasev deleted the 915_enforce_yapf branch February 8, 2021 17:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enforce code formatting repo-wide and add CI job
3 participants