Skip to content

Added <messy> tag for JSHint maven plugin...#147

Open
bradproctor wants to merge 1 commit into
alexo:1.7.xfrom
bradproctor:1.7.x
Open

Added <messy> tag for JSHint maven plugin...#147
bradproctor wants to merge 1 commit into
alexo:1.7.xfrom
bradproctor:1.7.x

Conversation

@bradproctor

Copy link
Copy Markdown

...to disregard "Mixed spaces and tabs." errors when set to true. Defaults to false to maintain normal functionality if it is not specified.

…and tabs." errors when set to true. Defaults to false to maintain normal functionality if it is not specified.
@alexo

alexo commented Aug 9, 2013

Copy link
Copy Markdown
Owner

Thanks for this PR. Few questions and remarks:

  • Doesn't jshint has already an option for ignoring mixed spaces?
  • is it safe to remove an item from collection while iterating on it?
  • is there an unit test which proves that this test case behave as expected?
  • is there an open issue which described the problem being fixed by this PR?

You might find it usefull to read the workingWithGit wiki page on google project page. When dealing with a large number of features, keeping this process consistent is very important.

Thanks,
Alex

@bradproctor

Copy link
Copy Markdown
Author

To address your questions:

  • JSHint does not allow this itself, and the developer has said that he will not be implementing it multiple times, despite it being one of the most common requests. There is a flag called smartTabs, but that only allows the mix if the indentation is done with tabs, followed by spaces for alignment, but still breaks if they are interspersed.
  • It is safe to remove items from a Collection while iterating as long as you use the Iterator's .remove() method to do so. It is the only safe way to do it.
  • There is not a unit test, I will write one.
  • There are no currently open issues that I could find regarding this fix. I had to make it for the projects I'm using it on due to a fairly diverse dev team, so I figured I'd contribute.

With regards to the workingWithGit page, I assume you mean that I should branch the 1.7.x branch into a feature branch, then submit a pul reuqest from that branch into the main repo's 1.7.x?

Thanks,
Brad

@alexo

alexo commented Aug 9, 2013

Copy link
Copy Markdown
Owner

That is correct. Creating feature branch is preferred since I usually merge these also into future major unrelesded stable branches (ex: 2.0.x). But this is not mandatory, since I can do it myself.

I am currently on holiday, so I'll probably perform the merge only in a week.

Thanks,
Alex

@alexo

alexo commented Aug 16, 2013

Copy link
Copy Markdown
Owner

I will perform the merge as soon as there are unit tests available which proves that this feature works as expected.

Thanks,
Alex

@bradproctor

Copy link
Copy Markdown
Author

Thank you. I am currently on vacation and will return at the end of the week, I will get the unit tests written as soon as I get home.

Thanks,
Brad

@alexo alexo modified the milestones: 1.7.5, 1.7.4 Mar 21, 2014
@alexo alexo modified the milestones: 1.7.6, 1.7.5 Apr 8, 2014
@alexo alexo modified the milestones: 1.7.7, 1.7.6 Jun 30, 2014
@alexo alexo force-pushed the 1.7.x branch 3 times, most recently from 4780bf9 to 2e5d0dd Compare September 2, 2014 10:55
@alexo alexo removed this from the 1.7.7 milestone Sep 25, 2014
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