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

For the purposes of negative globs, treat them as positive globs. #67

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

krainboltgreene
Copy link
Contributor

In Github you can define a "negative glob" aka, "not this path". To still validate this type of glob we simply check the opposite of that path.

In Github you can define a "negative glob" aka, "not this path". To still validate this type of glob we simply check the opposite of that path.
@mpalmer
Copy link
Owner

mpalmer commented Jan 6, 2024

Hi Kurtis, good catch. I've never used a negative glob myself, so I guess that's why they don't work. 😂

Clippy's got a bone to pick with you -- although I would have expected a regular build to fail, too, so I'll ponder what's going on there some other time. If you can add a couple of tests to the pile in test (maybe 003a_successful_negative_globs and 004a_failing_negative_globs?) that'll let me just click the "Merge" button when all the checks are green.

@krainboltgreene
Copy link
Contributor Author

Yeah I'm not sure what the problems here are, as all the documentation details this function as taking 2 arguments.

…replace is called?

This is my second line of rust code ever written.
@krainboltgreene
Copy link
Contributor Author

oh, lol there's a replace function on Option: https://doc.rust-lang.org/std/option/enum.Option.html#method.replace

@mpalmer
Copy link
Owner

mpalmer commented Jan 8, 2024

Thanks for the updates, Kurtis. I'll get onto this by next week, I promise. 🤞

@krainboltgreene
Copy link
Contributor Author

@mpalmer Any chance you could run the actions?

@mpalmer
Copy link
Owner

mpalmer commented Jan 14, 2024

Sorry, I thought once I'd approved them once they'd be run automatically thereafter. TIL...

@mpalmer mpalmer mentioned this pull request Jan 30, 2024
@mpalmer mpalmer merged commit 91ad200 into mpalmer:main Jan 30, 2024
12 of 16 checks passed
@krainboltgreene krainboltgreene deleted the patch-1 branch January 30, 2024 00:40
@mpalmer
Copy link
Owner

mpalmer commented Jan 30, 2024

I created a second PR, #71, based on yours, and fixed up a few things. It's now merged, so we can consider this done, I reckon. Thanks for your work to make it happen!

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