nblockchain / conventions Goto Github PK

View Code? Open in Web Editor NEW

1.0 1.0 10.0 1.85 MB

License: MIT License

TypeScript 38.85% F# 61.05% Shell 0.10%

conventions's People

Contributors

Stargazers

Watchers

Forkers

tehraninasab parhamsaremi knocte aarani kubaflo raelmarv webwarrior-ws 0xgoerlimainnet mersho

conventions's Issues

Adopt "prettier" for our TypeScript code

In the same way we use fantomless/fantomas for F# code, we should use Prettier tool for TypeScript.

FTR: https://www.npmjs.com/package/prettier

PS: Let's not use https://github.com/marketplace/actions/prettier-action , we don't want to depend on GitHubActions marketplace.

wrapLatestCommitMsg.fsx doesn't seem to work in Windows

My latest commit msg is:

GitHubCI: move release creation outta linux job

Given that the asset upload fails often, and needs to be retried;
then it's better to put release creation out of the job that
needs to be retried (otherwise it fails with "release already
exists", and then the release needed to be deleted manually
before the retry).

First line is over 64 chars and I call wrapLatestCommit.fsx and nothing changes.

Allow paragraphs or lists in body, not a mix of them

Test that should pass (exit code 0):

This is a title

Foo bar baz.

Bar baz foo.

Test that should pass (exit code 0):

This is a title

- Foo bar baz.
- Bar baz foo.

Test that should pass (exit code 0):

This is a title

* Foo bar baz.
* Bar baz foo.

Test that should fail (exit code !== 0):

This is a title

Foo bar baz.
Bar baz foo.

wrapLatestCommitMsg script failed ignoring a code block

The script doesn't ignore the code block in the following commit message:

scripts/styleChecker.fsx: add

Add styleChecker.fsx script to check style of our F#, TS
and YML codes.

The `git restore package.json` command in the
styleChecker.fsx script was failing with the following
error, even when I was running the
`git config --global --add safe.directory '*'` command in
both the styleChecker.fsx script and in the CI. In the
issue [1], it's suggested by someone to use --system
instead of --global in the mentioned command, when the
git command is running in a container, which solved the
problem.

```
fatal: detected dubious ownership in repository at '/__w/conventions/conventions'
To add an exception for this directory, call:

	git config --global --add safe.directory /__w/conventions/conventions

Error when running 'git restore package.json'
Fsdk.Process+ProcessFailed: Exception of type 'Fsdk.Process+ProcessFailed' was thrown.
   at Fsdk.Process.ProcessResult.Unwrap(String errMsg)
   at Fsdk.Process.ProcessResult.UnwrapDefault()
   at FSI_0002.RunPrettier(String arguments)
   at <StartupCode$FSI_0002>.$FSI_0002.main@()
Stopped due to error
Error: Process completed with exit code 1.
```

[1] https://github.com/actions/checkout/issues/1048

BOM setting in .editorconfig?

See editorconfig/editorconfig#297 (comment)

Easy dotnet conventions to verify

Find "" instead of String.Empty/string.Empty
Find a .?proj file Foo, find the folder it's in is not named Foo.
Find a src/ folder, find folder Foo under it, find .fs/.cs file that doesn't contain word Foo (for namespace).
Find a src/ folder, find folder Foo under it, find folder Bar under Foo, find .fs/.cs file in Bar that doesn't contain word Foo.Bar (for namespace).
Find <OutputType>Exe</OutputType> in a .?proj file, then find Console. in some .cs/.fs file, then find project file (and folder for it) is not suffixed as Console.
Don't find <OutputType>Exe</OutputType> in a .?proj file, then find Console. in some .cs/.fs file.
Don't find <OutputType>Exe</OutputType> in a .fsproj file, then find Async.RunSynchronously in some .fs file.

Suggestions are not found in a case-insensitive way

Example: "Upgrade foo" (where foo exceeds 50 chars). The error should have recommended "upgrade" -> "update", but it didn't.

Create a script that compares commit-msg with PR

This script would check how many commit the PR has:

If more than 1 commit, it would abort (return ExitCode 0).
If only 1 commit, it would check:
- If PR title and commit msg title differ: fail.
- If PR description and commit msg body differ: fail.

Detect if some naming conventions are respected

See lines 27 and 28 of: c38b3fd

footer-notes-misplacement false positive

From https://github.com/nodeeffect/RunIntoMe/actions/runs/5471278315/jobs/9962255316:

npx commitlint --from HEAD~1 --to HEAD --verbose
⧗   input: GitHubCI: rollback prettier/plugin-xml to 2.2.0

We had a CI regression [1] when formatting XAML files:

```
sudo npm install --save-dev prettier @prettier/plugin-xml
added 5 packages in 898ms
[warn] Ignored unknown option --xml-whitespace-sensitivity=ignore. Did you mean --html-whitespace-sensitivity?
Error:  No parser could be inferred for file "/__w/RunIntoMe/RunIntoMe/src/Frontend/AddFriendPage.xaml".
...
```

[1] https://github.com/nodeeffect/RunIntoMe/actions/runs/5466042118/jobs/9959987069
✖   Footer messages must be placed after body paragraphs, please move any message that starts with "Fixes", "Closes" or "[i]" to the end of the commmit message.
For reference, these are the guidelines that include our commit message conventions: https://github.com/nblockchain/conventions/blob/master/WorkflowGuidelines.md [footer-notes-misplacement]

spawnSync is not cross-platform

Steps To Reproduce:

On Windows: try to call the "runCommitLintOnMsg" function or debug a test to see the function results.

Current Results:

'spawnSync npx ENOENT'

workaround (not recommended) :
https://stackoverflow.com/a/37138541

Exepected Results:

Commitlint should be executed without errors.

body-soft-max-line-length exception

This rule should not break the build if the offending line exceeds 64 chars because it uses a commit hash at the end.

Testcase:

GrpcService: fix some logging nits

These mistakes were made in 45faeca2f0e7c9c5545f54fb3fcc815f52b8a7cf.

Testcase that should still break the build (because line is longer than 64 chars before the commit hash):

GrpcService: fix some logging nits

These mistakes were made in this GrpcService's RunIntoMeService commit: 45faeca2f0e7c9c5545f54fb3fcc815f52b8a7cf.

footer-refs-validity false positive

+ npx commitlint --from HEAD~1 --to HEAD --verbose
⧗   input: scripts/checkCommits1by1: fix IndexOutOfRangeEx

We tried to make this script support for legit absence
of CI status via [1] but that caused IndexOutOfRangeExs.
This change now makes sure that the commitMsg is extracted
from the 1st API call (which is not dependent on a CI
status being present) so it should fix the problem (the
use of `[0]` hardcoded indexing was a big smell here that
turned out to cause this problem).

[1] 8c00fed7d2[45](https://github.com/knocte/conventions/actions/runs/7692618873/job/20959909590#step:7:46)89d7b70957cc872b1a1e57a76f93
✖   All references in the body must be mentioned in the footer, and vice versa.
For reference, these are the guidelines that include our commit message conventions: https://github.com/nblockchain/conventions/blob/master/docs/WorkflowGuidelines.md [footer-refs-validity]

footer-notes-misplacement false positive

See https://github.com/nodeeffect/RunIntoMe/actions/runs/4797635782/jobs/8534872563

Culprit: https://github.com/nodeeffect/RunIntoMe/commit/a643823f492c7a54cff35e70c1cdc30aa17da4cc

body-paragraph-line-min-length false positive

From https://github.com/nodeeffect/RunIntoMe/actions/runs/5471263849/jobs/9962228401:

npx commitlint --from HEAD~1 --to HEAD --verbose
⧗   input: GitHubCI: rollback prettier/plugin-xml to 2.2.0

We had a CI regression [1] when formatting XAML files:
```
sudo npm install --save-dev prettier @prettier/plugin-xml
added 5 packages in 898ms
[warn] Ignored unknown option --xml-whitespace-sensitivity=ignore. Did you mean --html-whitespace-sensitivity?
Error:  No parser could be inferred for file "/__w/RunIntoMe/RunIntoMe/src/Frontend/AddFriendPage.xaml".
...
```

[1] https://github.com/nodeeffect/RunIntoMe/actions/runs/5466042118/jobs/9959987069
✖   Please do not subceed 50 characters in the lines of the commit message's body paragraphs (except the last line of each paragraph); we recommend this script (for editing the last commit message): 
https://github.com/nblockchain/conventions/blob/master/scripts/wrapLatestCommitMsg.fsx
For reference, these are the guidelines that include our commit message conventions: https://github.com/nblockchain/conventions/blob/master/WorkflowGuidelines.md [body-paragraph-line-min-length]

Easy to find C# & F# & TS pitfalls

e.g. for C#: \s+async\s+void\s+ -> replace with async Task

e.g. for TypeScript: undefined -> replace with ? or null

e.g. for F#:
* \s+task\s*{ -> replace with async (and Async.StartAsTask if needed)
* let! _ = -> replace with do!

Improve scripts/detectNotUsingGitPush1by1.fsx to detect broken(red) CI statuses

After detectNotUsingGitPush1by1.fsx checks that gitPush1by1 was used it should check CI status of each commit:

If commit msg contains text "failing test", then it would be allowed to have broken CI.
Else: CI should be green (and if it's not, then script should exit with ExitCode>0).

wrapCommitMsg script failed testcase

It introduces extra EOL when wrapping this msg:

Frontend: fixed app data reset for Android (#34)

Auto Backup for Apps is a feature of Android 6.0 (API level 23)
and later that backs up user's app data (shared preferences,
files in the app's internal storage, and other specific files).
Data is restored when an app is reinstalled or installed on a
new device. Therefore, to make the app behave the same on both
Android and iOS platforms, and to be able to test welcomePage
easily after uninstalling&reinstalling the app, this option has been
disabled.

From https://github.com/nodeeffect/RunIntoMe/actions/runs/5392489462/jobs/9790909756

body-paragraph-min-line false positive (list items)

Testcase:

Merge PR https://github.com/NodeEffectLtd/RunIntoMe/pull/36 from raelmarv/wip/moving-english-strings-into-texts

* i18n: moved English strings to a common place.
* Frontend,i18n: replaced the word friend with folk.

NOTE: this new Frontend version requires a new backend
deployment.

Mmm, probably * and - at the beginning of a line should be considered a paragraph each, even if there's no 2-EOL separation between them.

Suggestions shouldn't be offered for the area

Testcase:

configure: this is the very very very very very long title

Error message should NOT recommend the "config" suggestion.

commitlint: footer-refs-validity bug

Code blocks should be allowed after a ref, testcase:

foo: bar baz

Blah blah [1].

[1]:
` ` `
System.IO.FileLoadException: Could not load file or assembly 'GWallet.Backend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'. The located assembly's manifest definition does not match the assembly reference. (0x80131040)
File name: 'GWallet.Backend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'
   at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMarkHandle stackMark, ObjectHandleOnStack assemblyLoadContext, ObjectHandleOnStack type, ObjectHandleOnStack keepalive)
   at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext)
   at System.RuntimeType.GetType(String typeName, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark)
   at System.Type.GetType(String typeName)
   at GWallet.Backend.Marshalling.ExtractType(String json) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Marshalling.fs:line 156
   at GWallet.Backend.Account.ImportSignedTransactionFromJson(String jsonOrCompressedJson) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Account.fs:line 713
   at GWallet.Backend.Account.LoadSignedTransactionFromFile(String filePath) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Account.fs:line 766
   at Program.BroadcastPayment() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line 62
   at Program.PerformOperation(UInt32 numActiveAccounts, UInt32 numHotAccounts) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line [39](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:40)8
   at Program.ProgramMainLoop[a]() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line [47](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:48)8
   at Program.NormalStartWithNoParameters() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line [49](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:50)0
` ` `

subject-full-stop false positive

When title ends with ... then subject-full-stop fails. We should probably fix this in commitlint (upstream).

Body prose: should rule be ignored if paragraph ends with URL?

See testcase:

Frontend.XF.Android: switch to SDK-style

Strangely enough when opening the new gwallet.android.sln in
VS4Mac, it asks the user to install the wasm-tools-net6.0
workload, even though CI works without it.

Original PR: https://github.com/nblockchain/geewallet/pull/190

Current results:
✖ Please begin a paragraph with uppercase letter and end it with a dot [body-prose]

wrapLatestCommitMsg.fsx doesn't seem to work empty commits

Steps to reproduce:

git commit -m "foo: testing
>
> This is a so long long long long long long long long long long long long long long long long long commmit" --allow-empty

dotnet fsi .\scripts\wrapLatestCommitMsg.fsx

result:

YOou asked to na mbernadn cthh ew impo/sfti xrWercaepnStc rciopmtmOintW,i nbduotw sd
oYionugr  sbor awnocuhl di sm aakhee
aidt  oefm p'tayn.d rYeosu/ wciapn/ friexpWeraatp Sycoruirp tcOonmWmiannddo wwsi't hb y- -2a lcloow-empty, orm myiotus .c
a n 
(ruesmeo v"eg itth ep ucsohm"m itto entirely  pwuibtlhi s"hg iyto urre sleotc aHlE AcDo^m"m.i
ts)

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        .vscode/
        package-lock.json

No changes

Error when running 'git commit --amend --message "foo: testing

This is a so long long long long long long long long long long
long long long long long long long commmit"'
Fsdk.Process+ProcessFailed: Exception of type 'Fsdk.Process+ProcessFailed' was thrown.
   at Fsdk.Process.ProcessResult.Unwrap(String errMsg)
   at Fsdk.Process.ProcessResult.UnwrapDefault()
   at <StartupCode$FSI_0005>.$FSI_0005.main@() in C:\Users\ASUS\source\repos\conventions\scripts\wrapLatestCommitMsg.fsx:line 60
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
Stopped due to error

expected result:

~~If the commit is empty, the script should modify the commit message using the --allow-empty option.~~

OS: Windows
Branch: Master

GitHubCI should check if commits of a PR include a Merge commit, and fail if yes

commitlint: footer-refs-validity bug

False positive:

Backend(,Tests): don't marshall hybrid type names

This is the 1st part (out of three) to fix bug 242: in certain
circumstances, the call to Type.GetType(fullTypeName) was
returning null (and then causing an NRE like the stacktrace in
bug 242 proves) or a FileLoadException [3].

It turns out that .NET's BCL, despite having separate
properties for the Type names "FullName" vs
"AssemblyQualifiedName", the first one still adds assembly
qualified names to types that are part of a generic type
definition. I.e., it will not add them in a type such as
"System.String" but it will in one such as "List<String>"
(see [1]):

` ` `
"System.Collections.Generic.List`1[[System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c5619[34](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:35)e089]]
` ` `

This could be dangerous because it would embed the version
number (even though we already have a JSON field for that in
our MarshallingWrapper type, which could make the type be not
found by Type.GetType (and as a consequence return null, or
throw the exception mentioned before).

The fix, or rather workaround, here, is to use ToString()
instead of the .FullName property, as hinted by this S.O.
answer: [2].

[1] https://learn.microsoft.com/en-us/dotnet/api/system.type.fullname
[2] https://stackoverflow.com/a/4662878/23158491
[3] Stack trace:
` ` `
System.IO.FileLoadException: Could not load file or assembly 'GWallet.Backend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'. The located assembly's manifest definition does not match the assembly reference. (0x80131040)
File name: 'GWallet.Backend, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'
   at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMarkHandle stackMark, ObjectHandleOnStack assemblyLoadContext, ObjectHandleOnStack type, ObjectHandleOnStack keepalive)
   at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext)
   at System.RuntimeType.GetType(String typeName, Boolean throwOnError, Boolean ignoreCase, StackCrawlMark& stackMark)
   at System.Type.GetType(String typeName)
   at GWallet.Backend.Marshalling.ExtractType(String json) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Marshalling.fs:line 156
   at GWallet.Backend.Account.ImportSignedTransactionFromJson(String jsonOrCompressedJson) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Account.fs:line 713
   at GWallet.Backend.Account.LoadSignedTransactionFromFile(String filePath) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Backend/Account.fs:line 766
   at Program.BroadcastPayment() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line 62
   at Program.PerformOperation(UInt32 numActiveAccounts, UInt32 numHotAccounts) in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line [39](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:40)8
   at Program.ProgramMainLoop[a]() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line [47](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:48)8
   at Program.NormalStartWithNoParameters() in /Users/knocte/Documents/Code/geewalletSTABLE/src/GWallet.Frontend.Console/Program.fs:line [49](https://github.com/nblockchain/geewallet/actions/runs/7526526463/job/20485327146#step:7:50)0
` ` `

Suggestions should not be offered for sub-words

Testcase:

Fix android build because blah blah very very very very very long title

Error message should NOT recommend the "and" -> "&" suggestion.

Check if gitPush1by1 was used

The way to do it:

For a PR (so, a CI step that has an if: github.event_name == 'pull_request'), a CI job should be triggered that checks if all commits in the PR have a CI status, using:
- This API: https://docs.github.com/en/rest/checks/suites
- The commits info: ${{ github.event.pull_request.head.sha }} and ${{ github.event.pull_request.commits }}

footer-refs-validity false positive

From https://github.com/nodeffectLtd/RunIntoMe/actions/runs/5530782872/jobs/10090619696. Commit msg: https://github.com/nodeffectLtd/RunIntoMe/commit/d4cc624cc95d237686e55ba6317efb9f0b791cfd

body-prose false positive

https://github.com/nblockchain/geewallet/actions/runs/5796723490/job/15711151832 from nblockchain/geewallet@6b3c64e:

Backend: adjust legacy obj path (#218)

Using the same folder for two projects (e.g.
GWallet.Backend.fsproj and GWallet.Backend-legacy.fsproj) that
use different frameworks (e.g. NetStandard2.0 one and .NET4.x
one), causes problems due to incompatible caches in obj folder,
so by adjusting the cache obj folder for the legacy version we
workaround this problem.

Result:

commitlint/[email protected]
+ npx commitlint --from HEAD~1 --to HEAD --verbose
⧗   input: Backend: adjust legacy obj path (#218)

Using the same folder for two projects (e.g.
GWallet.Backend.fsproj and GWallet.Backend-legacy.fsproj) that
use different frameworks (e.g. NetStandard2.0 one and .NET4.x
one), causes problems due to incompatible caches in obj folder,
so by adjusting the cache obj folder for the legacy version we
workaround this problem.
✖   The body of the commit message (as opposed to the commit message title) is composed of paragraphs. Please begin each paragraph with an uppercase letter and end it with a dot (or other valid character to finish a paragraph).
For reference, these are the guidelines that include our commit message conventions: https://github.com/nblockchain/conventions/blob/master/WorkflowGuidelines.md [body-prose]

nblockchain / conventions Goto Github PK

conventions's People

Contributors

Stargazers

Watchers

Forkers

conventions's Issues

Steps To Reproduce:

Current Results:

Exepected Results:

Steps to reproduce:

result:

expected result:

Recommend Projects

Recommend Topics

Recommend Org