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

Updated nuspec for Dnn #5

Merged
merged 6 commits into from
May 18, 2018
Merged

Updated nuspec for Dnn #5

merged 6 commits into from
May 18, 2018

Conversation

SkyeHoefling
Copy link

Since we are now going to be using NuGet to manage our dnn branded ClientDependency project we have updated the nuspec file to give it a unique Dnn Name.

Fixes: #3
Related: dnnsoftware/Dnn.Platform#2061

@tpluscode
Copy link

Please also update the artifact path in appveyor.yml

@SkyeHoefling
Copy link
Author

I'm not able to get the appveyor.yml script working locally, I think this may have to do with the appveyor.yml script is missing from the master branch. Does anyone have more experience with appveyor that can confirm this before I spend too much time debugging this build?

@tpluscode
Copy link

Don't worry about running locally. Just push and let's have Appveyor verify it's good.

@SkyeHoefling
Copy link
Author

SkyeHoefling commented May 18, 2018

I understand this is a small fork that we use specifically for Dnn Platform and we need to get stuff working, but I do not agree with committing changes before testing them locally or being able to certify the change is working.

I am going to give this another shot and see what I can get working. There are some things that I really want to test in the appveyor.yml file that don't look right to me. I would like to make sure that it works when we merge this pull request

@SkyeHoefling
Copy link
Author

Everything appears to be working and the appveyor.yml file is getting picked up correctly by the build. In my fork I had to set the dnn branch as the default branch since it was using the master branch. I learned something new tonight :)

Since it was discussed in #3 that we don't have access to the Dnn MyGet server I recommend we configure the AppVeyor project NuGet feed. This is really easy to configure as it is pretty much configured for us.

Please make sure the NuGet screen looks like this sample below:
image

If you would like update the feed name to be branded for Dnn.ClientDependency. Either way I need to know what the project feed is so we can finish updating the Dnn Platform

@@ -7,7 +7,8 @@ image: Visual Studio 2017
nuget:
project_feed: true
disable_publish_on_pr: true

version: 1.9.2.{build}

Choose a reason for hiding this comment

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

I specifically removed this because it will cause duplicate build numbers. Besides, it will have no effect on the outcome because the build number is then changed programmatically using the Update-AppveyorBuild command. It doesn't hurt either but it feels redundant to have this and ProductVersion variable below

Copy link
Author

Choose a reason for hiding this comment

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

The build number wasn't working for me on my fork so I added it in to certify it was working from the config file

@tpluscode
Copy link

The MyGet feed is already set up as AppVeyor's Environment. I will push as soon as this PR is merged.

https://www.myget.org/F/dnn-software-public/api/v2/package

If you would like to test beforehand, I enabled the Appveyor's project feed, named it https://ci.appveyor.com/nuget/dnn.clientdependency.

I do not agree with committing changes before testing them locally or being able to certify the change is working.

Is running appveyor.yml locally an official feature? I never heard about it? I'm sorry if you misunderstood me. In the end, when doing CI config, the only real way is to verify on the CI. And that's why there are pull requests, right? To verify changes are working before integrating with others :).

I would like to make sure that it works when we merge this pull request

Which one were those?

@SkyeHoefling
Copy link
Author

@tpluscode just to clarify with your first point here, when we complete a merged pull request is there a manual process or is it automatically going to push to the MyGet.

Just as an aside there is a discussion happening back in #3 about publishing this to NuGet.org. I think we can move forward with MyGet to unblock us on dnnsoftware/Dnn.Platform#2061 and if we do need to move the final destination we can later.

To address your question about running appveyor.yml locally. I wasn't technically running it locally I was running it on my branch. Since appveyor is free for all open source projects I could run it on my fork of this fork to test if the scripts were actually going to work. I was struggling getting the appveyor.yml to be picked up because my fork had the default branch set to master and not dnn. Once I switched that over I was very quickly able to resolve the problem.

Looks like we need to fix something with the nuspec file. I am going to take a 2nd look later today. I just checked the project feed and do not see the proper name for the nuget. See screenshot below
image

@SkyeHoefling
Copy link
Author

I had some more time to dig through this and figure out why it wasn't showing up in the project feed. Notice the version numbers

Name Version
Screenshot v1.9.2.6
AppVeyor v1.9.2.7

It appears the build didn't publish the nupkg to the project feed since this is a pull request which makes sense based on the changes to the appveyor file.

@tpluscode I think we are good to merge this Pull Request since there is nothing left to test.

@tpluscode
Copy link

just to clarify with your first point here, when we complete a merged pull request is there a manual process or is it automatically going to push to the MyGet.

Manual. I don't think it's worth automating, especially that AppVeyor acts weird around tags. This will likely be a very rare occurrence.

I think we can move forward with MyGet to unblock us

💯 agreed. Merging now. Thanks!

@tpluscode tpluscode merged commit 11baed6 into dnnsoftware:dnn May 18, 2018
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.

2 participants