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 todo spec test for libsass issue 2994 #1473

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

mgreter
Copy link
Contributor

@mgreter mgreter commented Oct 4, 2019

@mgreter mgreter merged commit 4f1944c into sass:master Oct 4, 2019
@mgreter mgreter deleted the todo/issue-2994 branch October 4, 2019 21:54
@nex3
Copy link
Contributor

nex3 commented Oct 10, 2019

This spec (along with many others you've landed over the past week) doesn't match the style guide and was landed without review. I've asked you before to avoid landing PRs like this, so I've switched on branch protection to require it from now on. Please update these specs to convert them to style guide format.

@mgreter
Copy link
Contributor Author

mgreter commented Oct 10, 2019

Yep, I did under the libsass directories, for all others I added you as reviewer. If that's your point, fine with me, means I wont contribute any more spec tests in the future. So be it that way.

@nex3
Copy link
Contributor

nex3 commented Oct 10, 2019

The ultimate goal of the style guide is to make every spec in the repository compliant, regardless of where it originated. This should produce a hierarchy of specs, organized by topic, that together cover the entire language. If we find that an implementation has a regression in behavior that the specs missed, we can add a test in a well-understood location that specifically targets that regression. We've already started doing so in cases like #1464.

When new specs are added that aren't style-guide-compliant and don't fit into the global spec hierarchy, it just means more work to eventually migrate those specs in the future. Specs that are fairly large chunks of real-world Sass code are particularly difficult to migrate, because it's not clear what specific piece of functionality they cover.

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.

2 participants