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 ., -, and _ to set of unescaped characters in urlencode #72

Merged
merged 3 commits into from Aug 21, 2022
Merged

Add ., -, and _ to set of unescaped characters in urlencode #72

merged 3 commits into from Aug 21, 2022

Conversation

lucky
Copy link
Contributor

@lucky lucky commented Aug 19, 2022

I ran make test and then cargo insta review to verify the changes. I'm not sure if there's anything else that's required. (See below for a note on a line that changed and I'm unsure why.)

The `.snap` file was added by confirming changes from within `cargo insta review`
@@ -1,9 +1,7 @@
---
source: minijinja/tests/test_templates.rs
assertion_line: 44
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why this changed

Copy link
Owner

Choose a reason for hiding this comment

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

I think your version of cargo-insta is probably too old.

Copy link
Owner

Choose a reason for hiding this comment

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

Actually in this case it's the other way round. Snapshots were made on an older insta version.

@lucky lucky changed the title Add . to set of unescaped characters in urlencode Add ., -, and _ to set of unescaped characters in urlencode Aug 19, 2022
@lucky
Copy link
Contributor Author

lucky commented Aug 19, 2022

@mitsuhiko I believe this is ready for review. I'm also interested in creating a PR for the README to add a section on how to develop for this repository. Any feedback here is helpful, thanks!

@mitsuhiko mitsuhiko merged commit b11289f into mitsuhiko:main Aug 21, 2022
@mitsuhiko
Copy link
Owner

Thank you for this. Happy to accept a PR for contributing. Ideally put the main contents into a CONTRIBUTING.md file. I made one for insta which could be a point of reference: https://github.com/mitsuhiko/insta/blob/master/CONTRIBUTING.md

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.

None yet

2 participants