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

add script #845

Closed
wants to merge 8 commits into from
Closed

add script #845

wants to merge 8 commits into from

Conversation

sharinetmc
Copy link
Contributor

No description provided.

@sharinetmc sharinetmc requested a review from shaunagm June 22, 2023 20:43
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

This looks good - the only issue is the linting. Did you run Black?

@sharinetmc sharinetmc requested a review from shaunagm June 26, 2023 19:03
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

Looks like linting is still failing?

@sharinetmc
Copy link
Contributor Author

sharinetmc commented Jun 27, 2023

Hmm...interesting. I just ran black again and it said that the file was left unchanged.

@shaunagm
Copy link
Collaborator

Can you share the full output of the Black command, as well as the location you're running the command from?

@sharinetmc
Copy link
Contributor Author

(parsons) ➜ parsons git:(sample_ngpvan_printed_list) black useful_resources/sample_code/ngpvan_sample_printedlist.py
All done! ✨ 🍰 ✨
1 file left unchanged.

@shaunagm
Copy link
Collaborator

So the instructions for how to use Black in the docs are wrong, whoops. Can you try black --check parsons/ test/ useful_resources/

…#1017)

* black and flake8 linter fixes

* Bump Multiple Docs Deps Around Sphinx (#975)

* Bump multiple docs deps

* Downgrade myst-parser

---------

Co-authored-by: Soren Spicknall <[email protected]>
@shaunagm
Copy link
Collaborator

shaunagm commented Apr 4, 2024

Thanks for the help @bmos but this PR is now including 3 extra files which it shouldn't need to touch. Any idea why/how to fix?

@bmos
Copy link
Contributor

bmos commented Apr 4, 2024

Afaik that is because it was needed to get the linting check to pass. You can see the checks on the two commits in my PR.
Log from before including the sphinx docs related commit: https://github.com/move-coop/parsons/runs/22737528175
Log from after including that commit: https://github.com/move-coop/parsons/runs/22737929377

@bmos
Copy link
Contributor

bmos commented Apr 4, 2024

Now that you have updated the branch to main, it's using the new tooling which targets a longer line-length.
I can re-format in the next few days.

@shaunagm
Copy link
Collaborator

shaunagm commented Apr 4, 2024

It's probably easier for Sharine to just redo with a new PR off the most recent main branch. It's easy enough since it's a single file, and then she can run Black which will use whatever the current standards are.

@sharinetmc
Copy link
Contributor Author

@shaunagm this is done! finally lol

@sharinetmc sharinetmc requested a review from shaunagm May 20, 2024 14:10
@sharinetmc sharinetmc closed this May 23, 2024
@sharinetmc sharinetmc deleted the sample_ngpvan_printed_list branch May 23, 2024 21:59
@shaunagm shaunagm mentioned this pull request May 23, 2024
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.

3 participants