search

chaijs / chaijs.github.io

The chaijs.com website source code. Contributions welcome.
http://chaijs.github.io
49 stars 71 forks source link

Ban some plugins from the plugin listing #136

Closed keithamus closed 8 years ago

keithamus commented 8 years ago

This bans 2 plugins. Anyone from @chaijs/chai-docs care to take a look?

astorije commented 8 years ago

(By the way, @keithamus, is the LGTM thing fixed, aka, can other members than you merge now?)

lucasfcosta commented 8 years ago

@astorije We both can merge this, we just need to have enough LGTMs before doing so. LGTM too.

astorije commented 8 years ago

I wasn't 👍 on this btw, I think it's better to try to fix said repos instead of maintaining a list of banned ones.

lucasfcosta commented 8 years ago

@astorije Ah, I'm sorry, I thought that as you've asked here if @keithamus was the only one who could merge this you were agreeing to merge this now until the issues you opened got fixed. I'm sorry for this, it was totally my fault. Ah, and just as tip for other people reading this: let's avoid using the keyword LGTM without explicitly meaning that a PR is approved, otherwise LGTM.co thinks it is an approval and it counts towards the minimum approval necessary to merge something.

Sorry again @astorije, I really thought you were 👍 on this.

astorije commented 8 years ago

Sorry again @astorije, I really thought you were 👍 on this.

No worries at all, it's no big deal, I'll open a PR removing these when the repos will merge/publish their changes ;-)

Ah, and just as tip for other people reading this: let's avoid using the keyword LGTM without explicitly meaning that a PR is approved, otherwise LGTM.co thinks it is an approval and it counts towards the minimum approval necessary to merge something.

Ah ah, I had no idea. I have never used that service before, it kind of triggers my OCD to see these orange/pending checks in all non-approved PRs :-P

keithamus commented 8 years ago

Hey, no big deal, we can remove them from the blacklist as and when the PRs @astorije submitted get merged. I'd personally prefer to see us do this to quickly remove the problem and remove confusion for users, and then address the problem properly. My manager would describe this as "tactical then strategic" (but I shudder at that).

astorije commented 8 years ago

Fully agree with you, @keithamus (including the shudder 😄). OK to ban things ASAP and try to have them clean up on their end too.