Skip to content

Replace Squeaky CI advice with custom advice#897

Merged
frankharkins merged 7 commits intomainfrom
FH/squeaky-advice
Feb 27, 2024
Merged

Replace Squeaky CI advice with custom advice#897
frankharkins merged 7 commits intomainfrom
FH/squeaky-advice

Conversation

@frankharkins
Copy link
Copy Markdown
Member

@frankharkins frankharkins commented Feb 27, 2024

Closes #838.

Replaces the message in CI with correct instructions for our repository. I've added a switch to remove the advice from the printed message, I think this is the neatest net logic change: frankharkins/squeaky@33e24f8.

Screenshot 2024-02-27 at 17 20 42

See frankharkins#2 for the test PR.

This doesn't display the message when running tox -e lint, but I think it's unlikely a contributor would run that command locally without first encountering it in CI.

@frankharkins frankharkins linked an issue Feb 27, 2024 that may be closed by this pull request
@frankharkins frankharkins marked this pull request as ready for review February 27, 2024 17:38
Copy link
Copy Markdown
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks!

Thoughts on the CI change so that Squeaky failure doesn't block uploading notebooks? That would allow this message to say to either run tox -e fix locally or to rely on CI. Totally fine to be in a follow up.

Comment thread .github/workflows/notebook-test.yml Outdated
Copy link
Copy Markdown
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks! Last thing to close #838 is mentioning in the README how to install tox, ideally with pipx.

Copy link
Copy Markdown
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks, Frank :)

@frankharkins
Copy link
Copy Markdown
Member Author

Thank you too!

@frankharkins frankharkins added this pull request to the merge queue Feb 27, 2024
Merged via the queue into main with commit 390829f Feb 27, 2024
@frankharkins frankharkins deleted the FH/squeaky-advice branch February 27, 2024 18:17
frankharkins added a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Closes Qiskit#838.

Replaces the message in CI with correct instructions for our repository.
I've added a switch to remove the advice from the printed message, I
think this is the neatest net logic change:
frankharkins/squeaky@33e24f8.


<img width="600" alt="Screenshot 2024-02-27 at 17 20 42"
src="https://github.com/Qiskit/documentation/assets/36071638/dff70ada-2eca-4041-83fd-e0177bc52bca">

See #2 for the test
PR.

This doesn't display the message when running `tox -e lint`, but I think
it's unlikely a contributor would run that command locally without first
encountering it in CI.

---------

Co-authored-by: Eric Arellano <[email protected]>
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.

Squeaky CI failures need more clear instructions

2 participants