search

FabricCommunity-Archive / community-mod-maybe

8 stars 9 forks source link

Extend the rules #4

Closed Chocohead closed 5 years ago

Chocohead commented 5 years ago

One man's massive class is another man's detailed texture

asiekierka commented 5 years ago

I was waiting for this, no joke

shedaniel commented 5 years ago

This is the best item mod

shedaniel commented 5 years ago

I wonder what is hidden

ChloeDawn commented 5 years ago

This pull request does not break any of the rules and is prime for merge 🚀

shedaniel commented 5 years ago

I still don't know what this code do lol

Shnupbups commented 5 years ago

I'm not gonna lie, I'm not entirely sure what this code does either, but judging by the mere fact that there's a class named 'LoopHoleHandler', I think it might violate rule 14 (in that it is an attempt to circumvent rules), and thus have to be rejected. (or, in this case, reverted)

I won't do it myself as, again, I'm still not sure what this does in fact do... but if someone could maybe... take another look at it?

asiekierka commented 5 years ago

(or, in this case, reverted)

Which rule says anything about reverting? :thinking:

ChloeDawn commented 5 years ago

There is not one massive class that does everything, there are 3 classes that have designated purposes. And there is no overuse of internal classes, they are all top-level classes.

Shnupbups commented 5 years ago

There is not one massive class that does everything, there are 3 classes that have designated purposes. And there is no overuse of internal classes, they are all top-level classes.

The rule is clearly worded so that those are examples of possible circumventions covered by the rule, not a list. Not the use of such as. "Attempts to circumvent these rules through means such as..."

Therefore it still falls under it. (Also, when I was writing the rules, I intended for 'internal classes' to just mean several classes in the same file. I'll make the rule more clear.)

(or, in this case, reverted)

Which rule says anything about reverting?

I think if a PR has been merged that shouldn't have been, it should be fixed. I don't think I seriously need to make a rule for that.

Shnupbups commented 5 years ago

Further comment on whether this gets reverted or not should be on #5 . Again, not gonna actually revert it myself, I've specifically requested others to review it.