Add "Reviewed by Hound" badge #6150
Conversation
salbertson
commented
Oct 17, 2018
•
salbertson
commented
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
2 similar comments
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
|
Do not merge! This patch has not been tested yet. Can an existing organization member please verify this patch? |
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
|
👎 from me for adding the hound badge, as we recently moved away from it for JS (because we can't use eslint plugins in it) and personally I'd also like to remove it for the Ruby code, mainly as houndci/hound#1250 is still unsolved after almost two years and also we can't use Rubocop plugins in hound. |
|
I personally would not mind having that in our README until we decide to remove it. It did decent job already, it is not perfect but I still find it useful. But that's not reason to refuse such patch. I also see there is something happening in the relevant PR which is a good sign already. @salbertson please remove the image and amend. This is far from being decided yet, we are a big team. |
|
@mmoll we are making progress on configuring linter versions, it's really close. houndci/hound#1250 (comment) |
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine. More guidelines are available in Coding Standards or on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
|
@salbertson nah, I am good, thanks. I created thread on our site to bring some attention to the discussion: https://community.theforeman.org/t/reviewed-by-hound/11512 |
|
@salbertson: Please see this threads for issues we have with hound: https://community.theforeman.org/t/replacing-hound-ci-for-the-js-stack/11116 The two most painful things in my opinion are:
|
|
@timogoebel awesome, thank you for the feedback! We are actively working on making linter versions configurable and the option to move comments out of the code changes, using GitHub's Checks interface.
Do you have any more detail or a good example for us to look into? |
|
I personally think that as long as we use hound (and we still do) we should have the badge in our readme, if / when we decide to move away from it, we should remove it too, 👍 to merge this PR. |
If you only check diffs then you assume a strictly linear history. Let's say you submit a PR X that has a certain coding style, submit & merge PR Y that changes the coding style and then merge PR X. Now you have a commit with a different coding style that could still have passed checks. |
|
@ohadlevy you want to comment on the discourse thread then? I think it's fair to give credit, folks already expressed they commitment to make sure the badge stays there for as long as possible fixing some bugs or doing enhancements which is great. I did not read those tho, don't have any details about it. |
With this commit, if "All users" is ticked then locations and organizations are added to users. On edit user page, you can not edit this taxonomy as they are disabled with tooltip "Select all option enabled for this taxonomy".
With this commit, if "All users" is ticked then locations and organizations are added to users. On edit user page, you can not edit this taxonomy as they are disabled with tooltip "Select all option enabled for this taxonomy".
With this commit, if "All users" is ticked then locations and organizations are added to users. On edit user page, you can not edit this taxonomy as they are disabled with tooltip "Select all option enabled for this taxonomy".
With this commit, if "All users" is ticked then locations and organizations are added to users. On edit user page, you can not edit this taxonomy as they are disabled with tooltip "Select all option enabled for this taxonomy".
With this commit, if "All users" is ticked then locations and organizations are added to users. On edit user page, you can not edit this taxonomy as they are disabled with tooltip "Select all option enabled for this taxonomy".
