search

nblockchain / conventions

MIT License
1 stars 10 forks source link

commitlint: build in strict mode #142

Closed Mersho closed 9 months ago

aarani commented 1 year ago

What's the plan here? Just adding all strict flags in TS compiler? Sorry but I hate it! This is not a spacecraft control panel. /cc @knocte

knocte commented 1 year ago

What's the plan here? Just adding all strict flags in TS compiler? Sorry but I hate it!

The plan is this: https://github.com/nblockchain/lnp2pbot/issues/2 (inspired by this: https://hackernoon.com/making-typescript-truly-strongly-typed )

knocte commented 1 year ago

package.json: add packages devDependencies

You're explaining the what, but not the why.

knocte commented 1 year ago

One way to fix this is to assign a function expression to a variable name instead:

AFAIU, your commit doesn't do this anymore.

knocte commented 10 months ago

@Mersho can I get an update here, what happened with this PR?

Mersho commented 10 months ago

@Mersho can I get an update here, what happened with this PR?

CI of the last commit failed on "installing dependencies". Also, the PR CI of the second commit failed but we don't have the logs. I'm going to investigate it now.

Mersho commented 10 months ago

last commit CI was failed because of commitlint warning:

✖ No need to use space after comma in the scope (so that commit title can be shorter).

everything is fine so you can merge.

Mersho commented 10 months ago

My branch CI is green but if you want a green PR CI we need to add 'failing test' to first commit:

please make sure that the commit message contains the 'failing test' term

knocte commented 10 months ago

everything is fine so you can merge.

How can you say that everything is fine if you just pointed out the (fixable) issue in CI? FIX THE CI! I won't merge a PR that has red CI.

knocte commented 10 months ago

The error below occurs when a function declaration is inside a block in strict mode when targeting ‘ES3’ or ‘ES5’. One way to fix this is to assign a function expression to a variable name instead:

commitlint/plugins.ts:19:26 - error TS1251: Function declarations are not allowed inside blocks

I disagree with the way you are fixing this!!! THE POINT OF USING THE STRICT FLAG IS TO BE MORE STRICT! If you add the strict flag but later add another flag that makes the strict flag weaker, WHAT IS THE POINT???? Just fix the goddamn compiler error!!!!!!!

knocte commented 10 months ago

@Mersho can you rebase this on Monday please

knocte commented 9 months ago

Also print the typescript version on the CI as well.

When you have to write the word "also" in a commit message, you're already suspicious of mixing things (in the same commit) that maybe shouldn't be mixed.

And in this case, in fact, I'm going to ask you to split that commit in two, and separate the devDependencies part to another PR please.