coreruleset / go-ftw

Web Application Firewall Testing Framework - Go version
Apache License 2.0
115 stars 29 forks source link

Remove magefile? #299

Open theseion opened 4 months ago

theseion commented 4 months ago

@fzipi I really don't like the magefile approach. It may be flexible but it doesn't play nice with pre-commit, for example. One issue that leads to is that we have a lot of commits where we reorder imports, depending on whether we ran mage or not.

Another issue: VS Code by default updates import order on save, so changing the order with mage and gosimport is only temporary.

fzipi commented 4 months ago

Well, I normally use Goland. How do we keep things consistent?

theseion commented 4 months ago

Not sure. I tried running mage as a pre-commit program but that doesn't really work because of auto staging. Honestly, pre-commit is fine but it's a pain if it's not in line with what the CI runs.

The CI runs:

pre-commit runs:

mage runs:

mage can also run (I've never done that):

IMO, most of what pre-commit does is good and should also be part of the CI. gosimports is probably better for us than goimports, so I'd also add that to the CI and remove goimports from pre-commit. addlicense should probably also go in the CI; not sure about gci.

We could of course do all that with mage and simply run mage in the CI. I'm not a fan but maybe that's the way to go.

fzipi commented 4 months ago

Have no problems either way.