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

Adds #916 #936

Closed
wants to merge 6 commits into from
Closed

Adds #916 #936

wants to merge 6 commits into from

Conversation

cfischer
Copy link

wrap_attributes: 'force',
wrap_attributes_indent_size: 'align',
reformat_unformatted_tags: true

@cfischer
Copy link
Author

Closes #916

@bitwiseman
Copy link
Member

Hello, I'm sorry for the delay. I've had no time to review changes or to fix the issue with the CI tools. Please be patient.

@bitwiseman bitwiseman closed this May 31, 2016
@bitwiseman bitwiseman reopened this May 31, 2016
@AppVeyorBot
Copy link

Build js-beautify 1.6+214 failed (commit 4915400091 by @cfischer)

@AppVeyorBot
Copy link

Build js-beautify 1.6+215 failed (commit 4915400091 by @cfischer)

@bitwiseman
Copy link
Member

It looks like we still have failures. Please take a look when you get a chance.

@bitwiseman
Copy link
Member

@cfischer - please take a look and add tests. You can run ./build to verify your feature is working at expected.

@Lukinoh
Copy link
Contributor

Lukinoh commented Sep 1, 2016

@bitwiseman - I did not see this pull request, and hence I also coded the 'force-align' feature (Lukinoh@3410529). But what I wanted to know is how it should be tested. Because, the test ecosystem seems to be too rigid.

Concretely, I wanted to add my tests to the 'wrap_attributes' tests (name: AttributeWrap), however the 'indent'_attr' value is fixed through all tests (for one entry matrix) and as we want to test something where the 'indent_attr' is dynamic for each test, the tests break.

What would be the right way to do ? Creating several specific groups just for this new attribute ? Something else ?

@bitwiseman
Copy link
Member

@LukinoS - Please create a new pull request and try adding one test where you think it should go (even if it fails). Then I'll be able to give you concrete feedback.

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.

4 participants