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.

Sign up for GitHub

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

Jump to bottom

Suppress code style warnings and use central package management #513

Merged
merged 3 commits into from
Mar 7, 2024
Merged

Suppress code style warnings and use central package management #513

merged 3 commits into from
Mar 7, 2024

Conversation

frg2089
Copy link
Contributor

@frg2089 frg2089 commented Feb 22, 2024 •
edited
Loading

Close #512 48046a6
Close #501 4e8a9a1

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 22, 2024 •
edited
Loading

Nightly build for this pull request:

@frg2089 frg2089 marked this pull request as draft February 26, 2024 12:57
@frg2089 frg2089 marked this pull request as ready for review February 26, 2024 13:33
SadPencil
SadPencil requested changes Mar 6, 2024
View reviewed changes
Copy link
Member

@SadPencil SadPencil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides this comment, I don't have other complaints.

build/AfterPublish.targets Outdated Show resolved Hide resolved
@frg2089
Use CPM CnCNet#512
a6deca3 
Update build.yml

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>
@frg2089
Remove StyleCop.Analyzers and looser code style limits
dca726f 
Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>
@frg2089 frg2089 requested a review from SadPencil March 6, 2024 01:54
@frg2089
Copy link
Contributor Author

frg2089 commented Mar 6, 2024

Should I fixed #515 in this PR?

@SadPencil
Copy link
Member

SadPencil commented Mar 6, 2024 •
edited
Loading

Should I fixed #515 in this PR?

I think not unless your want your PR being reviewed for months

@SadPencil SadPencil changed the title Used Central Package Management Suppress code style warnings and use central package management Mar 6, 2024
@MahBoiDeveloper
Copy link
Contributor

Warning NU1903 seems to be critical one.

@frg2089
Copy link
Contributor Author

frg2089 commented Mar 6, 2024 •
edited
Loading

Warning NU1903 seems to be critical one.

We cannot fix it now.
Because the ImageSharp 3 are not supported .Net Framework.
We must have another solution to replace ImageSharp.
such as SkiaSharp.
It should be resolved in another PR.

Note

SkiaSharp only supported .jpg, .png and .webp formats.

@SadPencil
Copy link
Member

SadPencil commented Mar 6, 2024 •
edited
Loading

Warning NU1903 seems to be critical one.

Just ignore it. The client only loads pictures from local files instead of Internet so it is pretty fine to ignore this high risk.

Obsolete. See messages below

@SadPencil
Copy link
Member

Warning NU1903 seems to be critical one.

We cannot fix it now. Because the ImageSharp 3 are not supported .Net Framework. We must have another solution to replace ImageSharp. such as SkiaSharp. It should be resolved in another PR.

Note

SkiaSharp only supported .jpg, .png and .webp formats.

https://github.com/SixLabors/ImageSharp/releases/v2.1.7

They released v2.1.7 to backport for .NET Framework an hour ago. Upgrading to this version should be fine

@Metadorius
Copy link
Member

Warning NU1903 seems to be critical one.

Just ignore it. The client only loads pictures from local files instead of Internet so it is pretty fine to ignore this high risk.

The upcoming MapDB 2.0 may change that status quo, so I am not sure if it's fine. We must also account possible future features too.

@SadPencil
Copy link
Member

Warning NU1903 seems to be critical one.

Just ignore it. The client only loads pictures from local files instead of Internet so it is pretty fine to ignore this high risk.

The upcoming MapDB 2.0 may change that status quo, so I am not sure if it's fine. We must also account possible future features too.

That message is obsolete since v2.1.7 is released

@Metadorius
Copy link
Member

The upcoming MapDB 2.0 may change that status quo, so I am not sure if it's fine. We must also account possible future features too.

That message is obsolete since v2.1.7 is released

That's fine, I just wanted to highlight the importance of foreseeing the future changes in general.

@Metadorius
Copy link
Member

Let's merge this after the ImageSharp upgrade that fixes the issue.

Metadorius
Metadorius reviewed Mar 7, 2024
View reviewed changes
Directory.Packages.props Outdated Show resolved Hide resolved
@Metadorius Metadorius merged commit 928f04c into CnCNet:develop Mar 7, 2024
3 checks passed
@frg2089 frg2089 deleted the feat/cpm branch March 7, 2024 12:36
SadPencil pushed a commit to SadPencil/xna-cncnet-client that referenced this pull request Oct 4, 2024
@frg2089 @Metadorius @SadPencil
Suppress code style warnings and use central package management (CnCN…
fb69049 
…et#513)

* Use CPM CnCNet#512

Update build.yml

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>

* Remove StyleCop.Analyzers and looser code style limits

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>

* Update ImageSharp

---------

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>
Co-authored-by: Kerbiter <[email protected]>
SadPencil pushed a commit to SadPencil/xna-cncnet-client that referenced this pull request Oct 4, 2024
@frg2089 @Metadorius @SadPencil
Suppress code style warnings and use central package management (CnCN…
be54716 
…et#513)

* Use CPM CnCNet#512

Update build.yml

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>

* Remove StyleCop.Analyzers and looser code style limits

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>

* Update ImageSharp

---------

Signed-off-by: 舰队的偶像-岛风酱! <[email protected]>
Co-authored-by: Kerbiter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Metadorius Metadorius Metadorius approved these changes

@SadPencil SadPencil SadPencil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use Central Package Management VS issue with building, and can't start after compiling - file not found
4 participants
@frg2089 @SadPencil @MahBoiDeveloper @Metadorius