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

Fix bug writing unknown tags #50

Merged
merged 3 commits into from
Jul 10, 2023
Merged

Conversation

simon-20
Copy link
Contributor

@simon-20 simon-20 commented Jun 5, 2023

This fixes the bug writing unknown tags. With this change, unknown tags will be written out as they were in the original file.

Copy link
Collaborator

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

Thanks for the submission - it looks like a valuable feature! Could you please write a test as well for this?

rispy/writer.py Outdated Show resolved Hide resolved
@simon-20
Copy link
Contributor Author

simon-20 commented Jun 5, 2023

Yes, I will do, on both counts. Should the make test command work/be working on Windows?

@shapiromatron
Copy link
Collaborator

Yes, I will do, on both counts. Should the make test command work/be working on Windows?

I dont think it'll work on windows. The makefile just calls pytest, so you can just call that directly:

rispy/Makefile

Lines 51 to 52 in d795f51

test: ## Run unit test suite
@py.test

@simon-20
Copy link
Contributor Author

simon-20 commented Jun 8, 2023

Yes, I will do, on both counts. Should the make test command work/be working on Windows?

I dont think it'll work on windows. The makefile just calls pytest, so you can just call that directly:

rispy/Makefile

Lines 51 to 52 in d795f51

test: ## Run unit test suite
@py.test

I did get the tests to work on Windows; I had a misconfiguration of make on my end.

Copy link
Collaborator

@shapiromatron shapiromatron left a comment

Choose a reason for hiding this comment

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

LGTM - great addition!

@shapiromatron shapiromatron merged commit 28c8c53 into MrTango:main Jul 10, 2023
4 checks passed
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

3 participants