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 Hound to staging to prevent any code linting #16956

Merged
merged 1 commit into from
Jul 4, 2017
Merged

Add Hound to staging to prevent any code linting #16956

merged 1 commit into from
Jul 4, 2017

Conversation

C-Lodder
Copy link
Member

@C-Lodder C-Lodder commented Jul 3, 2017

As requested by @mbabker here: #16947 (comment)

@wilsonge
Copy link
Contributor

wilsonge commented Jul 3, 2017

On a serious note do we want to enable js stuff? I get that our current js isn't compliant but I don't see a reason to disable it either. This also needs to be removed from Joomla builds (I.e. Add it to the array of excluded files in build/build.php)

@C-Lodder
Copy link
Member Author

C-Lodder commented Jul 3, 2017

I'm not bothered if it's enabled or disabled on staging. Release manager can decide.

Had to do the build.php as a separate PR: #16957

@C-Lodder C-Lodder mentioned this pull request Jul 3, 2017
@dgrammatiko
Copy link
Contributor

On a serious note do we want to enable js stuff?

Sure! BUT we need to review all files (in a sprint or something) otherwise whenever anyone creates a new PR will be facing a lot of failures for strict comparisons etc., which will cause a lot of confusion. I guess this is a task for the JS group: @Fedik @dneukirchen @yvesh

@C-Lodder
Copy link
Member Author

C-Lodder commented Jul 4, 2017

If you want to enable JS linting, then this PR doesn't need to be merged.

@photodude
Copy link
Contributor

I would say merge with the expectation that a sprint will take care of the current failures in the JS code then enable JS linting on staging as @dgt41 suggested.

@C-Lodder
Copy link
Member Author

C-Lodder commented Jul 4, 2017

@photodude - That's double the work. We've rewritten most of the JS in 4.0. Is it really worth wasting resources linting on both branches?

@mbabker
Copy link
Contributor

mbabker commented Jul 4, 2017

That's double the work. We've rewritten most of the JS in 4.0. Is it really worth wasting resources linting on both branches?

IMO no

@wilsonge wilsonge merged commit c1271e3 into joomla:staging Jul 4, 2017
@wilsonge wilsonge added this to the Joomla 3.7.4 milestone Jul 4, 2017
@C-Lodder C-Lodder deleted the patch-2 branch July 4, 2017 16:30
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

6 participants