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.

Sign up for GitHub

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

Jump to bottom

Fix the tests #10

Merged
merged 38 commits into from
Dec 4, 2020
Merged

Fix the tests #10

merged 38 commits into from
Dec 4, 2020

Conversation

peternewman
Copy link
Collaborator

@peternewman peternewman commented Jun 3, 2020 •
edited
Loading

For the new normalised hidden file behaviour, this shouldn't be merged until version 1.18.0 or greater is released.

@peternewman peternewman mentioned this pull request Jun 9, 2020
Merged
@peternewman peternewman added this to the Codespell-2.0 milestone Sep 1, 2020
@peternewman peternewman requested a review from larsoner December 2, 2020 17:28
@larsoner
Copy link
Member

larsoner commented Dec 2, 2020

I see you requested review, feel free to ping me once things are green

@peternewman
Copy link
Collaborator Author

I see you requested review, feel free to ping me once things are green

Yeah sorry I forgot/hadn't looked at the BATS stuff failing.

@peternewman
Copy link
Collaborator Author

Okay @larsoner all the required tests are green again now. I've left the bats diagnostic thing in as it can help in future and just left it as not required, but I could remote it completely for now if you'd rather?

@larsoner
Copy link
Member

larsoner commented Dec 4, 2020

I've left the bats diagnostic thing in as it can help in future and just left it as not required, but I could remote it completely for now if you'd rather?

This makes it harder to review. I'd at least get it to "fail gracefully" so that things are green instead of part-red -- for example, returning a "neutral" rather than "failed" or "succeeded" status would be preferable

larsoner
larsoner approved these changes Dec 4, 2020
View reviewed changes
Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Other than not liking the "red on some is okay to merge" LGTM

@peternewman
Copy link
Collaborator Author

This makes it harder to review. I'd at least get it to "fail gracefully" so that things are green instead of part-red -- for example, returning a "neutral" rather than "failed" or "succeeded" status would be preferable

There's not really a neutral in GitHub Actions compared to Travis' Allowed Failures. Best bet would be as I'd done with it not being required so I've skipped it for now.

@peternewman peternewman merged commit 7cdfde6 into master Dec 4, 2020
@peternewman peternewman deleted the peternewman-fix-tests branch January 31, 2021 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larsoner larsoner larsoner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Codespell-2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@peternewman @larsoner