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

Port tests to haml 6 #4395

Closed
wants to merge 2 commits into from
Closed

Port tests to haml 6 #4395

wants to merge 2 commits into from

Conversation

terceiro
Copy link
Contributor

Fixes: #4382

@mojavelinux
Copy link
Member

Thanks for taking this on!

We will still need to test again Haml 5 for compatibility reasons. Would you be able to add conditionals in such a way that the tests work with either version? I'll then make sure the CI matrix is set up to run on both major versions.

@terceiro
Copy link
Contributor Author

Thanks for taking this on!

We will still need to test again Haml 5 for compatibility reasons. Would you be able to add conditionals in such a way that the tests work with either version? I'll then make sure the CI matrix is set up to run on both major versions.

There.

@terceiro terceiro force-pushed the haml-6 branch 3 times, most recently from bcb54bf to c36087b Compare January 2, 2023 22:46
@mojavelinux
Copy link
Member

Thanks! I'll review as soon as you think it's ready to go. Just give me the signal.

@terceiro
Copy link
Contributor Author

terceiro commented Jan 3, 2023

this should be ready. I tested against haml 5 and 6 by hacking the Gemfile, and it works

@thesamesam
Copy link

thesamesam commented Apr 7, 2023

Thanks for this, I was just trying to figure out why the tests were failing for us :)

@mojavelinux any chance you're able to review?

@mojavelinux
Copy link
Member

Superseded by #4431.

I don't want to introduce an intermediary class to add support for Haml 6, and I don't think it's necessary. Instead, I want to use the existing infrastructure in the template converter to deal with option variance. However, this PR did point me to what needed to be changed, so I very much appreciate the contribution.

I tested against haml 5 and 6 by hacking the Gemfile, and it works

We can't just rely on local testing. I added a profile to the CI workflow to run the test suite using Haml 5 in additional to Haml 6.

@mojavelinux mojavelinux closed this Apr 9, 2023
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.

Tests not working properly
3 participants