Easy dotnet conventions to verify by tehraninasab · Pull Request #139 · nblockchain/conventions

tehraninasab · 2023-08-16T08:55:50Z

Working on #122.

Add tests for DefiningEmptyStringsWithDoubleQuotes function.

Implement DefiningEmptyStringsWithDoubleQuotes function.

Add tests for ProjFilesNamingConvention function.

Implement ProjFilesNamingConvention function.

Add tests for NotFollowingNamespaceConvention function.

Implement NotFollowingNamespaceConvention function.

Add more tests for NotFollowingNamespaceConvention fn.

Fix NotFollowingNamespaceConvention function.

Add one more test for NotFollowingNamespaceConvention function.

Function DoesNamespaceInclude doesn't working on *.cs namespace.

parhamsaremi · 2023-08-18T07:44:14Z

src/FileConventions.Test/FileConventions.Test.fs

+[<Test>]
+let DefiningEmptyStringsWithDoubleQuotes1() =
+    let fileInfo =
+        (FileInfo(


extra parens

parhamsaremi · 2023-08-18T07:44:31Z

src/FileConventions.Test/FileConventions.Test.fs

+[<Test>]
+let DefiningEmptyStringsWithDoubleQuotes2() =
+    let fileInfo =
+        (FileInfo(


extra parens

parhamsaremi · 2023-08-18T07:47:37Z

src/FileConventions.Test/FileConventions.Test.fs

+[<Test>]
+let ProjFilesNamingConvention2() =
+    let fileInfo =
+        (FileInfo(


Extra paren.

parhamsaremi · 2023-08-18T07:48:04Z

src/FileConventions.Test/FileConventions.Test.fs

+            )
+        ))
+
+    Assert.That(ProjFilesNamingConvention fileInfo, Is.EqualTo true)


Isn't it easier to use Assert.True?

no, Assert.That is fine

parhamsaremi · 2023-08-18T07:48:16Z

src/FileConventions.Test/FileConventions.Test.fs

+[<Test>]
+let ProjFilesNamingConvention1() =
+    let fileInfo =
+        (FileInfo(


Extra paren

parhamsaremi · 2023-08-18T07:49:00Z

src/FileConventions.Test/FileConventions.Test.fs

+            )
+        ))
+
+    Assert.That(ProjFilesNamingConvention fileInfo, Is.EqualTo false)


Assert.False?

parhamsaremi · 2023-08-18T07:49:59Z

src/FileConventions/Library.fs

 let ProjFilesNamingConvention(fileInfo: FileInfo) =
-    printfn "%s" fileInfo.FullName
-    false
+    let regex = new Regex("(.*)\..*proj$")


no need for new keyword

also extra paren.

parhamsaremi · 2023-08-18T07:50:58Z

src/FileConventions/Library.fs

-    printfn "%s" fileInfo.FullName
-    false
+    let regex = new Regex("(.*)\..*proj$")
+    assert (regex.IsMatch fileInfo.FullName)


not sure about this but I think paren is not needed

it is needed; because assert doesn't receive 2 currified arguments

assert is for debug mode only, are we sure it should be used here?

it is needed; because assert doesn't receive 2 currified arguments

Not sure because it compiles without them 🤔

@aarani you're right, they should use Fsdk's Misc.BetterAssert

parhamsaremi · 2023-08-18T07:51:50Z

src/FileConventions.Test/FileConventions.Test.fs

+            )
+        ))
+
+    Assert.That(NotFollowingNamespaceConvention fileInfo, Is.EqualTo true)


Assert.True?

parhamsaremi · 2023-08-18T07:52:07Z

src/FileConventions.Test/FileConventions.Test.fs

+[<Test>]
+let NamespaceConvention1() =
+    let fileInfo =
+        (FileInfo(


extra paren

parhamsaremi · 2023-08-18T07:52:15Z

src/FileConventions.Test/FileConventions.Test.fs

+[<Test>]
+let NamespaceConvention2() =
+    let fileInfo =
+        (FileInfo(


extra paren

parhamsaremi · 2023-08-18T07:52:32Z

src/FileConventions.Test/FileConventions.Test.fs

+            )
+        ))
+
+    Assert.That(NotFollowingNamespaceConvention fileInfo, Is.EqualTo false)


Assert.False?

parhamsaremi · 2023-08-18T07:55:35Z

src/FileConventions/Library.fs

 let NotFollowingNamespaceConvention(fileInfo: FileInfo) =
-    printfn "%s" fileInfo.FullName
-    false
+    assert (fileInfo.FullName.EndsWith(".fs"))


extra paren

knocte · 2023-08-28T04:41:02Z

src/FileConventions/FileConventions.fsproj

  <ItemGroup>
    <PackageReference Include="Mono.Posix" Version="7.1.0-final.1.21458.1" />
    <PackageReference Include="Mono.Unix" Version="7.1.0-final.1.21458.1" />
+		<PackageReference Include="Fsdk" Version="0.6.0--date20230821-0702.git-5488853" />


@Mersho same here

knocte · 2023-08-28T04:42:00Z

src/FileConventions/Library.fs

+
+let ymlAssertionError = "Bug: file should be a .yml file"
+let projAssertionError = "Bug: file should be a proj file"
+let fsxAssertionError = "Bug: file was not a F#/C# source file"


@Mersho if it's an error about F# or C# files, then why do you call it "Fsx Assertion Error"?

Using RemoveEmptyEntries for split & removing extra parans & Regex does not require a new keyword & string formatting has been corrected.

Add tests for NotFollowingConsoleAppConvention() function.

Ensure that projects that aren't console applications don't have source files with console methods. Co-authored-by: Parham <parhaamsaremi@gmail.com>

Add tests for NotFollowingConsoleAppConvention function and we fix ConsoleAppConvention2 test so that project name does not contradict new tests.

Fix NotFollowingConsoleAppConvention() function.

Add tests for async project, Async.RunSynchronously only allowed in console applications.

Fix NotFollowingConsoleAppConvention() function.

Add test for NotFollowingNamespaceConvention fn.

A .fs/fsx file might not have a namespace sometimes.

`Contains()` method catches a lot of false-positives and not suitable for this situation so we used regex. Co-authored-by: Parham <parhaamsaremi@gmail.com>

Fix `DefiningEmptyStringsWithDoubleQuotes()` function by using regex insted of `Contains()`. Co-authored-by: Parham <parhaamsaremi@gmail.com>

Finding printf and console methods in files & removed printf methods used for debugging purposes & added file filter to ReturnAllProjectSourceFile.

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

knocte · 2023-08-31T08:44:00Z

@Mersho CI is still red

Mersho · 2023-08-31T09:13:09Z

@Mersho CI is red because it's failed to install nodejs

I don't have access to re-run CI, do you think I should re-push?

installing : node-v18.17.1
mkdir : /usr/local/n/versions/node/18.17.1
fetch : https://nodejs.org/dist/v18.17.1/node-v18.17.1-linux-x64.tar.gz
curl: (92) HTTP/2 stream 0 was not closed cleanly: INTERNAL_ERROR (err 2)

gzip: stdin: unexpected end of file
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now

  Error: failed to download archive for 18.17.1

Error: Process completed with exit code 1.

knocte · 2023-08-31T09:17:11Z

I don't have access to re-run CI,

Ask Zahra to give you access. The telegram channel is meant for this king of things.

do you think I should re-push?

Next time you face a problem like this, just repush while you wait for someone to give you perms; you shouldn't need to depend on me for this kind of small things :)

knocte · 2023-08-31T09:18:02Z

Next time you face a problem like this

BTW, I say "Next time" because I'm going to re-run that CI now, but this shouldn't mean that you don't need to ask for Zahra's perms anymore.

knocte · 2023-08-31T10:14:20Z

@Mersho the next CI issue might be a permissions problem because Zahra changed her username, I think. Let's workaround this by just opening a new PR from your account.

Mersho · 2023-08-31T13:29:53Z

The pull request has been superseded by a newer one

tehraninasab added 8 commits August 17, 2023 12:29

FileConventions: add failing test

50fe215

Add tests for DefiningEmptyStringsWithDoubleQuotes function.

FileConventions: implement the function

186213e

Implement DefiningEmptyStringsWithDoubleQuotes function.

FileConventions: add failing test

2ebd2b5

Add tests for ProjFilesNamingConvention function.

FileConventions: implement the function

cc82850

Implement ProjFilesNamingConvention function.

FileConventions: add failing test

a0272d6

Add tests for NotFollowingNamespaceConvention function.

FileConventions: implement the function

482d4d6

Implement NotFollowingNamespaceConvention function.

FileConventions.Test: add failing test

5f06394

Add more tests for NotFollowingNamespaceConvention fn.

FileConventions: fix the function

12f4d0b

Fix NotFollowingNamespaceConvention function.

Mersho force-pushed the verifyDotnetConventions-squashed branch 2 times, most recently from 756f364 to 275d6d1 Compare August 17, 2023 13:43

Mersho added 4 commits August 17, 2023 17:26

FileConventions: add failing test

b0acb0c

Add one more test for NotFollowingNamespaceConvention function.

FileConventions: fix csharp namespace

7823ab2

Function DoesNamespaceInclude doesn't working on *.cs namespace.

FileConventions: add failing test

9b8afb3

FileConventions: Contains() is not suitable here

21f3749

Mersho force-pushed the verifyDotnetConventions-squashed branch 3 times, most recently from 21f3749 to b0acb0c Compare August 17, 2023 14:01

parhamsaremi reviewed Aug 18, 2023

View reviewed changes

knocte reviewed Aug 28, 2023

View reviewed changes

Mersho force-pushed the verifyDotnetConventions-squashed branch from 1342e1d to 6e74482 Compare August 28, 2023 09:53

Mersho and others added 9 commits August 29, 2023 12:43

FileConventions(.Test): applying f# standard style

e124a91

Using RemoveEmptyEntries for split & removing extra parans & Regex does not require a new keyword & string formatting has been corrected.

FileConventions: add failing test

eb5f76c

Add tests for NotFollowingConsoleAppConvention() function.

FileConventions: fix the function

885a60a

Ensure that projects that aren't console applications don't have source files with console methods. Co-authored-by: Parham <parhaamsaremi@gmail.com>

FileConventions: add failing test

4e0c6e1

Add tests for NotFollowingConsoleAppConvention function and we fix ConsoleAppConvention2 test so that project name does not contradict new tests.

FileConvention: fix the function

6e80056

Fix NotFollowingConsoleAppConvention() function.

FileConventions: add failing test

46f3dae

Add tests for async project, Async.RunSynchronously only allowed in console applications.

FileConvention: fix the function

45e298b

Fix NotFollowingConsoleAppConvention() function.

FileConventions(.Test): fix proj file indent

c190f9c

FileConvention: properly naming variables

d49a99c

Mersho force-pushed the verifyDotnetConventions-squashed branch from 83803e8 to e124a91 Compare August 29, 2023 09:15

Mersho and others added 4 commits August 29, 2023 13:34

FileConventions: add failing test

19b442d

Add test for NotFollowingNamespaceConvention fn.

FileConvention: fix the function

e6fc879

A .fs/fsx file might not have a namespace sometimes.

FileConventions.Test: add failing test

5358b96

`Contains()` method catches a lot of false-positives and not suitable for this situation so we used regex. Co-authored-by: Parham <parhaamsaremi@gmail.com>

FileConventions: fix empty string false-positives

ce4c0c6

Fix `DefiningEmptyStringsWithDoubleQuotes()` function by using regex insted of `Contains()`. Co-authored-by: Parham <parhaamsaremi@gmail.com>

Mersho force-pushed the verifyDotnetConventions-squashed branch from 10b55f8 to 5358b96 Compare August 29, 2023 11:34

Mersho added 3 commits August 29, 2023 15:05

FileConventions: load Helpers.fs first

18b828e

FileConventions: improvements to Library.fs

1fd7576

Finding printf and console methods in files & removed printf methods used for debugging purposes & added file filter to ReturnAllProjectSourceFile.

FileConventions: dotnetFileConvention

a326d09

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

Mersho force-pushed the verifyDotnetConventions-squashed branch from 8fb77c7 to 18b828e Compare August 29, 2023 11:36

knocte closed this Aug 31, 2023

Mersho mentioned this pull request Aug 31, 2023

Easy dotnet conventions to verify #141

Closed

Conversation

tehraninasab commented Aug 16, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knocte commented Aug 31, 2023

Uh oh!

Mersho commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knocte commented Aug 31, 2023

Uh oh!

knocte commented Aug 31, 2023

Uh oh!

knocte commented Aug 31, 2023

Uh oh!

Mersho commented Aug 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Mersho commented Aug 31, 2023 •

edited

Loading