nblockchain / conventions

MIT License
1 stars 10 forks source link

Easy dotnet conventions to verify #141

Open Mersho opened 1 year ago

Mersho commented 1 year ago

Working on #122

Supersedes #139

knocte commented 1 year ago

@Mersho 2 things:

knocte commented 1 year ago

@Mersho I can see you have pushed something different from the previous PR, what did you change? (I can tell by seeing that the commit hashes are not the same)

Mersho commented 1 year ago

@Mersho I can see you have pushed something different from the previous PR, what did you change? (I can tell by seeing that the commit hashes are not the same)

@knocte I'm just re-pushing and amending, nothing has changed

git commit --amend --no-edit

knocte commented 1 year ago

please put the link of the old PR in the description, e.g. saying "Supersedes #XY"

You didn't do this yet.

Mersho commented 1 year ago

please put the link of the old PR in the description, e.g. saying "Supersedes #XY"

You didn't do this yet.

Done, I've edited the first comment.

knocte commented 1 year ago

Done, I've edited the first comment.

That's not the first comment, it's the PR description :)

knocte commented 1 year ago

FileConventions: improvements to Library.fs

Ideally, the changes that this commit brings should be moved to the corresponding commits that introduced those functions, to avoid git-blame pollution.

Mersho commented 1 year ago

FileConventions: improvements to Library.fs

Ideally, the changes that this commit brings should be moved to the corresponding commits that introduced those functions, to avoid git-blame pollution.

That's true, I'd like to squash the commits if it's okay with you

There are also some old commits that need to be squashed FileConventions(.Test): fix proj file indent FileConvention: properly naming variables

knocte commented 1 year ago

I'd like to squash the commits if it's okay with you

That's very ambiguous, what commits do you intend to squash? FYI I don't want you to squash everything into 1 commit.

Mersho commented 1 year ago

I'd like to squash the commits if it's okay with you

That's very ambiguous, what commits do you intend to squash? FYI I don't want you to squash everything into 1 commit.

nvm, it's fine.

knocte commented 1 year ago

FileConventions: dotnetFileConvention

This commit msg title is very rough, why doesn't it have a verb? It must summarize well what is happening, and this does not.

The new dotnetFileConvention.fsx script detects empty strings, wrong namespace usage, and incorrect printf methods on non-console applications.

dotnetFileConvention? You're clearly saying that this script is taking care of various conventions, not just one, so then the name of the script should be in plural, not singular.

Mersho commented 1 year ago

FileConventions: dotnetFileConvention

This commit msg title is very rough, why doesn't it have a verb? It must summarize well what is happening, and this does not.

The new dotnetFileConvention.fsx script detects empty strings, wrong namespace usage, and incorrect printf methods on non-console applications.

dotnetFileConvention? You're clearly saying that this script is taking care of various conventions, not just one, so then the name of the script should be in plural, not singular.

I've changed the commit message. Thank you for informing me.