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

convert nuget packages to use central package management #660

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

arturcic
Copy link
Member

@arturcic arturcic commented Jan 9, 2025

Convert the nuget package management to the central package management

@arturcic arturcic requested a review from gep13 January 9, 2025 19:02
@arturcic arturcic force-pushed the feature/cpm-nuget branch 2 times, most recently from 3947215 to d69e520 Compare January 9, 2025 20:35
Comment on lines 50 to 52
2.1.x
3.1.x
5.0.x
Copy link
Member

Choose a reason for hiding this comment

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

I am as sure as I can be that this is going to break some of the tooling that we have in this build, specifically the usage of Wyam for documentation generation, so I don't think we can remove these just now.

@gep13
Copy link
Member

gep13 commented Jan 9, 2025

@arturcic can I ask how you made this changes in this PR?

The reason that I am asking is that your changes seem to have the same whitespace/line endings changes that we are seeing in the Dependabot PR's. I am not seeing these changes when I edit the files on my machine, so there must be something that is causing this to happen.

What are you using in terms of:

  • Operating System
  • IDE
  • git version
  • etc

If we can compare these things, it might help isolate what is going on.

Thanks

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

I used Visual Studio in a VM on my Mac (meaning it's a Windows ARM64). I used the Upgrade Assistent extensions in Visual Studio to migrate to Central Package Management.
git version 2.44.0.windows.1

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

I will keep only 1 commit for Central Package Management ( was trying to do more in this PR, but probably step by step approach would be better)

@gep13
Copy link
Member

gep13 commented Jan 9, 2025

I am still hestitant to pull in these changes, because as soon as I do, I will get issues on my machine.

I really want to get to the bottom of this, as this will turn into a back and forth as we each make changes.

I have asked for some help from some people that might know more.

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

sure no worries, please update this PR as you need, the idea is to centralize the package management and thus have the dependabot PRs changing only one file.

@gep13
Copy link
Member

gep13 commented Jan 9, 2025

To rule something out, can you navigate to the folder where you have GRM cloned to, and then run the following:

git config core.autocrlf

What do you get back?

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

git config core.autocrlf

The result is true

@mkevenaar
Copy link

Do you both use Editorconfig plugins? see .editorconfig

@gep13
Copy link
Member

gep13 commented Jan 9, 2025

Yes, there is an editorconfig file in play here, but I don't believe that this is part of the issue. The changes that I am seeing are happening outside of an IDE. @AdmiringWorm and I went through and modified both the editorconfig file and gitattibutes file today, so they should be correct (or at least, as correct as we know about 😄)

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

Do you both use Editorconfig plugins? see .editorconfig

I installed Visual Studio from scratch on that VM, added only the Upgrade Assistant
image, I guess I'm using the VS defaults

@gep13
Copy link
Member

gep13 commented Jan 9, 2025

git config core.autocrlf

The result is true

I am not convinced that this is part of the problem, since the entry in the .gitattributes file should take care of this, but just so that I point it out:

@mkevenaar
Copy link

Do you both use Editorconfig plugins? see .editorconfig

I installed Visual Studio from scratch on that VM, added only the Upgrade Assistant

image, I guess I'm using the VS defaults

Try installing or configuring that tool and recreate these changes. See if that solves the line endings.

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

Do you both use Editorconfig plugins? see .editorconfig

I installed Visual Studio from scratch on that VM, added only the Upgrade Assistant
image, I guess I'm using the VS defaults

Try installing or configuring that tool and recreate these changes. See if that solves the line endings.

which tool? where I can get it from?

@mkevenaar
Copy link

mkevenaar commented Jan 9, 2025

Do you both use Editorconfig plugins? see .editorconfig

I installed Visual Studio from scratch on that VM, added only the Upgrade Assistant

image, I guess I'm using the VS defaults

Try installing or configuring that tool and recreate these changes. See if that solves the line endings.

which tool? where I can get it from?

https://learn.microsoft.com/en-us/community/content/how-to-enforce-dotnet-format-using-editorconfig-github-actions#3---formatting-your-code-locally

Editorconfig is installed by default in VS2022. Just check if it's enabled on your end

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

@gep13 you ca try to install that extension on your machine and select one of the projects and Right-click>Upgrade and selectimage

@arturcic
Copy link
Member Author

arturcic commented Jan 9, 2025

You might see the same changes, or if a bit different you can update this PR with your version

@AdmiringWorm
Copy link
Member

I had a look through the changed files, and from the looks of it there are no changes to any line endings, unlike what was previously said.

The changes has to do with the change from 4 spaces in the indentation to 2 spaces, and it probably have confused the web diff because of it.

There could be two reasons for the indentation change:

  1. The Upgrade Assistant tool does not respect the editorconfig file (last time I used it, it ignored the file).
  2. You initially had visual studio open with the old editorconfig file, then later switched to a new branch or updated commit without reloading VS (I have seen this happen from time to time that VS wouldn't refresh according to changes in the editorconfig).

@gep13 gep13 force-pushed the feature/cpm-nuget branch 2 times, most recently from 802cf48 to cbcec46 Compare January 10, 2025 19:51
@gep13 gep13 force-pushed the feature/cpm-nuget branch from cbcec46 to 93e778b Compare January 10, 2025 19:53
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Jan 10, 2025

@arturcic I have fixed this up locally, fixing the whitespace descrepancies.

Thanks for starting the work on this!

Going to get this merged in.

@gep13 gep13 added this to the 0.19.0 milestone Jan 10, 2025
@gep13 gep13 added the Build label Jan 10, 2025
@gep13 gep13 enabled auto-merge January 10, 2025 19:55
@arturcic
Copy link
Member Author

@arturcic I have fixed this up locally, fixing the whitespace descrepancies.

Thanks for starting the work on this!

Going to get this merged in.

Sure, you're welcome

@gep13 gep13 merged commit 443d62c into GitTools:develop Jan 10, 2025
2 checks passed
@arturcic arturcic deleted the feature/cpm-nuget branch January 10, 2025 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants