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

Fix nuget sanitycheck #236

Merged
merged 5 commits into from
Oct 20, 2023
Merged

Conversation

Mersho
Copy link
Contributor

@Mersho Mersho commented Oct 17, 2023

Closes #229
Closes #230
Closes #231
Closes #232

@Mersho
Copy link
Contributor Author

Mersho commented Oct 17, 2023

I'm gonna push1by1...

scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
@Mersho Mersho force-pushed the SanityCheckSquashed branch from 8ed3b3f to 58505a2 Compare October 17, 2023 09:03
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
@knocte
Copy link
Member

knocte commented Oct 17, 2023

You should place the commit titled scripts/sanitycheck: throw if sln doesn't exist as the 1st in the PR, otherwise I don't see its effects in CI.

@knocte
Copy link
Member

knocte commented Oct 17, 2023

The title "Sanity check file location changes" is very confusing because it doesn't really summarize the PR properly. I think the main issue that makes it very ambiguous is the absence of any verb in it.

@Mersho Mersho force-pushed the SanityCheckSquashed branch 2 times, most recently from 6804b72 to 6586f26 Compare October 17, 2023 09:09
@Mersho Mersho changed the title Sanity check file location changes scripts/sanitycheck.fsx: sln files updated Oct 17, 2023
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
@knocte
Copy link
Member

knocte commented Oct 17, 2023

BTW please beware of your type chenges -> changes.

@knocte
Copy link
Member

knocte commented Oct 17, 2023

BTW please beware of your type chenges -> changes.

Typo: type->typo

scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
scripts/sanitycheck.fsx Outdated Show resolved Hide resolved
@Mersho Mersho force-pushed the SanityCheckSquashed branch from acf9007 to 01bc478 Compare October 17, 2023 10:45
@knocte
Copy link
Member

knocte commented Oct 17, 2023

Ok, let's squash properly.

@Mersho Mersho force-pushed the SanityCheckSquashed branch 2 times, most recently from 03e8743 to 8c9c38c Compare October 17, 2023 11:07
@knocte
Copy link
Member

knocte commented Oct 17, 2023

I did NOT say squash commits into 1.

@Mersho Mersho force-pushed the SanityCheckSquashed branch 6 times, most recently from 6a355ba to 34658f0 Compare October 17, 2023 11:50
@Mersho Mersho force-pushed the SanityCheckSquashed branch 2 times, most recently from b2bf37f to 303cab8 Compare October 17, 2023 11:59
@Mersho Mersho changed the title scripts/sanitycheck.fsx: sln files updated Bring back the sanity check Oct 17, 2023
@knocte
Copy link
Member

knocte commented Oct 18, 2023

Bring back the sanity check

This PR title would make a future developer think that we had disabled or removed the sanity check and you brought it back. But this is not the case. The sanity check is/was there, you're just fixing one of its steps. Please fix the PR title.

@Mersho Mersho changed the title Bring back the sanity check Fix sanitycheck.fsx script Oct 18, 2023
@knocte
Copy link
Member

knocte commented Oct 19, 2023

Frontend.XF.*: sync & resolve packages conflict

This commit upgrades packages in Frotend.XF.* projects
to be in sync with versions used in Backend.

Sanity check was also complaining before this commit with
the following error:

Also? The error you're quoting is exactly the same as the thing you're referring to before that paragraph. It is not two different problems, so I think you should remove the word also.

@knocte
Copy link
Member

knocte commented Oct 19, 2023

Fix sanitycheck.fsx script

The sanitycheck script has many steps. Not all of them were broken. Please re-read (paying attention to detail) what I said in #236 (comment)

@knocte
Copy link
Member

knocte commented Oct 19, 2023

scripts/sanitycheck.fsx: fix dotnet legacy

dotnet legacy??? That sounds so weird and looks like a contradiction, given that in .NET4.x (legacy), the command dotnet doesn't exist. Please change the above to read .NET legacy to avoid any possible confusions.

@Mersho Mersho force-pushed the SanityCheckSquashed branch from 2407759 to ff08275 Compare October 19, 2023 12:47
@Mersho Mersho changed the title Fix sanitycheck.fsx script Readjustment sanitycheck.fsx script Oct 19, 2023
@knocte knocte force-pushed the SanityCheckSquashed branch from 307afc1 to e83032d Compare October 20, 2023 11:21
@knocte
Copy link
Member

knocte commented Oct 20, 2023

Readjustment sanitycheck.fsx script

Why did you remove the word "fix" from the title? I did not have a complain about that word. Please read again my previous complaint @Mersho .

@knocte knocte force-pushed the SanityCheckSquashed branch from ff2a8b7 to bdac2f1 Compare October 20, 2023 13:09
Mersho and others added 4 commits October 20, 2023 21:34
This got broken when moving solutions to sub /src/ folder [1].

[1] 03db5e5
This commit upgrades packages in Frotend.XF.* projects
to be in sync with versions used in Backend.

Sanity check was also complaining before this commit with
the following error:

```
190 nuget packages found for solution gwallet.mac-legacy.sln
Package found with more than one version: JsonRpcSharp. All occurrences:
* Version: 0.98.0--date20230731-1252.git-6788e32. Dependency holder: GWallet.Backend/
* Version: 0.98.0--date20230731-1252.git-6788e32. Dependency holder: GWallet.Backend/
* Version: 0.94.0--date20201018-1119.git-05d4006. Dependency holder: GWallet.Frontend.XF.Android/
* Version: 0.94.0--date20201018-1119.git-05d4006. Dependency holder: GWallet.Frontend.XF.iOS/
Package found with more than one version: Newtonsoft.Json. All occurrences:
* Version: 13.0.2. Dependency holder: GWallet.Backend.Tests/
* Version: 13.0.2. Dependency holder: GWallet.Backend/
* Version: 13.0.2. Dependency holder: GWallet.Backend/
* Version: 11.0.2. Dependency holder: GWallet.Frontend.XF.Android/
* Version: 11.0.2. Dependency holder: GWallet.Frontend.XF.iOS/
```

Closes nblockchain#229
Closes nblockchain#230
Closes nblockchain#231
Closes nblockchain#232

Co-authored-by: Afshin Arani <[email protected]>
As a result of this change, the sanity check works with both
dotnet and mono, and the check is based on the environment
variables that we set on make.sh.

Co-authored-by: Afshin Arani <[email protected]>
@knocte knocte force-pushed the SanityCheckSquashed branch from bdac2f1 to c00f9e4 Compare October 20, 2023 14:01
@knocte knocte changed the title Readjustment sanitycheck.fsx script Fix nuget sanitycheck Oct 20, 2023
@knocte knocte merged commit 6a9163f into nblockchain:master Oct 20, 2023
15 checks passed
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