Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scripts to apply and check the style of F#, C#, TS, YMl and XAML files #103

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tehraninasab
Copy link
Contributor

@tehraninasab tehraninasab commented Apr 4, 2023

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

The git respore 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] actions/checkout#1048

scripts/styleChecker.fsx Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from f3730c7 to 18c175b Compare April 4, 2023 10:31
scripts/styleChecker.fsx Outdated Show resolved Hide resolved
scripts/styleChecker.fsx Outdated Show resolved Hide resolved
@knocte
Copy link
Member

knocte commented Apr 4, 2023

Please change GitHubCI to use this script also.

@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 3 times, most recently from 3339696 to 67cd86e Compare April 4, 2023 11:36
@tehraninasab
Copy link
Contributor Author

Please change GitHubCI to use this script also.

Done

@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from 67cd86e to 07c3a8f Compare April 4, 2023 12:01
scripts/styleChecker.fsx Outdated Show resolved Hide resolved
.github/workflows/CI.yml Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 5 times, most recently from 296f06f to 396169b Compare April 5, 2023 12:38
@knocte
Copy link
Member

knocte commented Apr 5, 2023

In the [1] issue

I guess you meant in the issue [1] ???

scripts/styleChecker.fsx Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from 396169b to 4c213d1 Compare April 5, 2023 12:47
@tehraninasab
Copy link
Contributor Author

In the [1] issue

I guess you meant in the issue [1] ???

Right

scripts/styleChecker.fsx Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 2 times, most recently from 6170a1c to 76f356d Compare April 5, 2023 12:55
@tehraninasab tehraninasab marked this pull request as ready for review April 5, 2023 12:55
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 2 times, most recently from fd95db8 to 1d801bf Compare April 5, 2023 13:21
scripts/styleChecker.fsx Outdated Show resolved Hide resolved
scripts/styleChecker.fsx Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from 1d801bf to 482347c Compare April 5, 2023 13:39
scripts/styleChecker.fsx Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 2 times, most recently from a88a403 to c185305 Compare August 3, 2023 12:23
@knocte
Copy link
Member

knocte commented Aug 4, 2023

Added some comments in tehraninasab@2ecf57b

@knocte
Copy link
Member

knocte commented Aug 4, 2023

And tehraninasab@c185305

@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from 2f45679 to c7b0b00 Compare August 7, 2023 10:22
@knocte
Copy link
Member

knocte commented Aug 8, 2023

Added another comment in c7b0b00

@knocte
Copy link
Member

knocte commented Aug 8, 2023

Added some comments in tehraninasab@2ecf57b

@realmarv please address the comments above. And stop working on another task before you finish this one please. I told you that you shouldn't start new tasks before you finish old ones, and you keep working on a new task that you decided to start some days ago. This PR needs to be merged first before you resume work on that.

BTW how is NX going? I don't see you chasing people to merge PRs and the like. (The fact that I assigned a task to Taras doesn't mean loss of focus on NX, I just gave him one extra low-prio task.)

@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from c7b0b00 to e79352e Compare August 8, 2023 09:37
@tehraninasab
Copy link
Contributor Author

Added another comment in c7b0b00

I don't see any comment there

@tehraninasab
Copy link
Contributor Author

Added some comments in realmarv@2ecf57b

@realmarv please address the comments above. And stop working on another task before you finish this one please. I told you that you shouldn't start new tasks before you finish old ones, and you keep working on a new task that you decided to start some days ago. This PR needs to be merged first before you resume work on that.

Sorry I thought I addressed those comments

@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from e2a80b2 to a35c6cb Compare August 8, 2023 09:54
@knocte
Copy link
Member

knocte commented Aug 8, 2023

I don't see any comment there

Do you see this one?: tehraninasab@2ecf57b

scripts/styleCheck.fsx Outdated Show resolved Hide resolved
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 2 times, most recently from ce5c213 to 3c55518 Compare August 8, 2023 10:15
@knocte
Copy link
Member

knocte commented Aug 8, 2023

Before I merge this PR, @aarani can you test the styleApply.fsx script independently?

@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from 3c55518 to 2ec433e Compare August 9, 2023 12:46
Add styleChecker.fsx script to check style of our F#, TS and
YML codes.

The `git respore 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] actions/checkout#1048
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch 3 times, most recently from 561f306 to 444ea54 Compare August 9, 2023 13:30
Split styleChecker into styleCheck and styleApply.
@tehraninasab tehraninasab force-pushed the addStyleCheckerScript-squashed branch from 444ea54 to 6a89659 Compare August 9, 2023 13:35
@aarani
Copy link
Contributor

aarani commented Aug 16, 2023

styleApply.fsx doesn't work on windows

PS C:\Users\afshi\source\repos\PDNA\PDNA> dotnet fsi C:\conventions\scripts\styleApply.fsx
npm list [email protected]
Fsdk.Process+ProcessCouldNotStart: Process could not start! Command: npm. Arguments: list [email protected].
 ---> System.ComponentModel.Win32Exception (2): An error occurred trying to start process 'npm' with working directory 'C:\Users\afshi\source\repos\PDNA\PDNA'. The system cannot find the file specified.
   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
   at Fsdk.Process.Execute(ProcessDetails procDetails, Echo echo)
   --- End of inner exception stack trace ---
   at Fsdk.Process.Execute(ProcessDetails procDetails, Echo echo)
   at FSI_0003.Helpers.InstallPrettier(String version) in C:\conventions\src\FileConventions\Helpers.fs:line 170
   at FSI_0004.FileConventions.StyleTypeScriptFiles() in C:\conventions\src\FileConventions\Library.fs:line 463
   at <StartupCode$FSI_0005>.$FSI_0005.main@() in C:\conventions\scripts\styleApply.fsx:line 16
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
Stopped due to error
PS C:\Users\afshi\source\repos\PDNA\PDNA> npm --version
9.6.7
PS C:\Users\afshi\source\repos\PDNA\PDNA>

@aarani
Copy link
Contributor

aarani commented Aug 16, 2023

Wrong commit msg: restore -> resprose

@aarani
Copy link
Contributor

aarani commented Aug 16, 2023

Ah also YMI -> YML

@knocte
Copy link
Member

knocte commented Aug 17, 2023

@parhamsaremi can you test running styleApply.fsx in your Linux OS (which I guess is ubuntu22.04.x) please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants