cleaning dependencies before update nuspec #84

ptrstpp950 · 2013-05-22T13:00:24Z

UpdateNuspec step does not clean dependencies before adding new one. Following commit fixed this.

jmarnold · 2013-05-22T14:03:42Z

@ptrstpp950 This one was actually on purpose:

"Each dependency is then written to the file as long as it does not explicitly exist in the file. "

However, I do think there is value in clearing out the dependencies. Maybe we make an OverrideFlag on the CreatePackagesInput? If that's true then we clear out the dependencies.

ptrstpp950 · 2013-05-22T15:12:35Z

Now when I run command ".\ripple.cmd create-packages -u" twice all dependiecies in nuspec are duplicated, which causes that nuspec file is invalid.

I can add OverrideFlag if you prefer, but still update should at least check if dependecies are already exist. And if yes update them instead of append new ones.

jmarnold · 2013-05-22T15:36:30Z

You're right, it's a combination of both. If override is specified, then it
updates the existing rather than appending. That's easy enough to add.

Do you feel like taking this one on? There's an existing test:
update_dependencies_while_creating_local_nugets that gives an example of
how to check the nuspec generation.

If you don't get to it, I can pick it up later today.

On Wed, May 22, 2013 at 10:12 AM, Piotr Stapp [email protected]:

Now when I run command ".\ripple.cmd create-packages -u" twice all
dependiecies in nuspec are duplicated, which causes that nuspec file is
invalid.

I can add OverrideFlag if you prefer, but still update should at least
check if dependecies are already exist. And if yes update them instead of
append new ones.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-18285237
.

ptrstpp950 · 2013-05-22T18:43:57Z

I added OverrideFlag, second part of this bug I will do tommorow

ptrstpp950 · 2013-05-22T20:39:39Z

I added fix for updating nuget dependecies, now it should be a complete solution. Unit tests are included. Please review :)

jmarnold · 2013-05-22T21:31:56Z

Thanks for doing this. I'll review later tonight.

On Wed, May 22, 2013 at 3:39 PM, Piotr Stapp [email protected]:

I added fix for updating nuget dependecies, now it should be a complete
solution. Unit tests are included. Please review :)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-18306281
.

ptrstpp950 · 2013-05-28T13:15:32Z

One more question: why when we specify overrde flag nuspec dependencies should be preserved?

jmarnold · 2013-05-28T13:16:56Z

The idea behind that one was for situations where the generation doesn't
work or isn't powerful enough. That way you could explicitly specify a
dependency and the generation would leave it alone.

On Tue, May 28, 2013 at 8:15 AM, Piotr Stapp [email protected]:

One more question: why when we specify overrde flag nuspec dependencies
should be preserved?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-18549351
.

ptrstpp950 · 2013-05-28T13:39:05Z

I see following assertions without Override flag. Invoked command in every assertion is:
.\ripple.cmd create-packages --update-dependencies --version 1.1

XXX exist in nuspec in version 1.0& XXX is a project. Result: update XXX in nuspec to 1.1
XXX does not exist in nuspec && XXX is a project. Result: add XXX in nuspec with version 1.1
XXX exist in nuspec in version 1.0 && XXX is not a project. Result: XXX is still in nuspec in version 1.0

Are you agree?

jmarnold · 2013-05-28T13:41:03Z

Yes, that looks correct to me.

On Tue, May 28, 2013 at 8:39 AM, Piotr Stapp [email protected]:

I see following assertions without Override flag. Invoked command in every
assertion is:
.\ripple.cmd create-packages --update-dependencies --version 1.1

XXX exist in nuspec in version 1.0& XXX is a project. Result: update
XXX in nuspec to 1.1

XXX does not exist in nuspec && XXX is a project. Result: add XXX in
nuspec with version 1.1

XXX exist in nuspec in version 1.0 && XXX is not a project. Result: XXX
is still in nuspec in version 1.0

Are you agree?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-18550763
.

ptrstpp950 · 2013-05-28T14:00:00Z

What about Override flag set to true
The command is .\ripple.cmd create-packages --update-dependencies --version 0.9 (version is lower than in nuspec)
Should assertion looks like:

XXX exist in nuspec in version 1.0& XXX is a project. Result: update XXX in nuspec to 0.9
XXX does not exist in nuspec && XXX is a project. Result: add XXX in nuspec with version 0.9
XXX exist in nuspec in version 1.0 && XXX is not a project. Result: XXX is not in nuspec.

Am I right? Especially look at point 3.

jmarnold · 2013-05-28T14:02:12Z

This SOUNDS right but I'm missing the context of the version constraints.
Remember that the version constraints are what inevitably determine to the
version that is generated for each dependency in the nuspec file. Those
constraints act upon the version you specify in the command.

On Tue, May 28, 2013 at 9:00 AM, Piotr Stapp [email protected]:

What about Override flag set to true
The command is .\ripple.cmd create-packages --update-dependencies
--version 0.9 (version is lower than in nuspec)
Should assertion looks like:

XXX exist in nuspec in version 1.0& XXX is a project. Result: update
XXX in nuspec to 0.9

XXX does not exist in nuspec && XXX is a project. Result: add XXX in
nuspec with version 0.9

XXX exist in nuspec in version 1.0 && XXX is not a project. Result: XXX
is still in nuspec in version 0.9

Am I right? Especially look at point 3.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-18552091
.

jmarnold · 2013-06-07T13:58:56Z

What's the status of this one?

ptrstpp950 · 2013-06-07T14:58:08Z

I did not have time. Sorry.

On Fri, Jun 7, 2013 at 3:58 PM, Josh Arnold [email protected]:

What's the status of this one?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-19108413
.

…g the again. Unit tests included

Scooletz · 2013-06-12T15:40:00Z

It's been a long conversation. I'll try to rephrase all the comments into one statement to make it easier to define what has to be done in this topic. A rebase of this branch will be published tomorrow to allow it to be applied (as soon as we get a consensus;-) ).

jmarnold · 2013-06-12T15:40:54Z

We might want to start over on this one. The tests were wrong the first
pass so it couldn't hurt to avoid the rebasing/merging conflicts and try
again.

On Wed, Jun 12, 2013 at 10:40 AM, Scooletz [email protected] wrote:

It's been a long conversation. I'll try to rephrase all the comments into
one statement to make it easier to define what has to be done in this
topic. A rebase of this branch will be published tomorrow to allow it to be
applied (as soon as we get a consensus;-) ).

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-19334362
.

ptrstpp950 · 2013-06-12T16:22:38Z

I hope Scooletz will find a way to clean this solution faster then me. I will try to help him.Josh Arnold [email protected] pisze:We might want to start over on this one. The tests were wrong the first
pass so it couldn't hurt to avoid the rebasing/merging conflicts and try
again.

On Wed, Jun 12, 2013 at 10:40 AM, Scooletz [email protected] wrote:

It's been a long conversation. I'll try to rephrase all the comments into
one statement to make it easier to define what has to be done in this
topic. A rebase of this branch will be published tomorrow to allow it to be
applied (as soon as we get a consensus;-) ).

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/84#issuecomment-19334362
.

—
Reply to this email directly or view it on GitHub.

jmarnold · 2013-09-18T05:34:28Z

Bringing this back to life via @SimonCropp:

"i think you should not be touching the nuspec. define everything on your own configs them merge them all together using a temp nuspec"
"dont allow people to define dependencies in the nuspec"
"it would also remove the side effect of doing a builds and then having a pending checkin since the nuspec has changed"

ptrstpp950 added 4 commits June 12, 2013 13:56

cleaning dependencies before update nuspec

9c3e62a

Add OverrideFlag to CreatePackagesInput

f63dd21

AddDependency fix for updating existing dependencies instead of addin…

22197bc

…g the again. Unit tests included

cleaning dependencies before update nuspec

cd2c413

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleaning dependencies before update nuspec #84

cleaning dependencies before update nuspec #84

ptrstpp950 commented May 22, 2013

jmarnold commented May 22, 2013

ptrstpp950 commented May 22, 2013

jmarnold commented May 22, 2013

ptrstpp950 commented May 22, 2013

ptrstpp950 commented May 22, 2013

jmarnold commented May 22, 2013

ptrstpp950 commented May 28, 2013

jmarnold commented May 28, 2013

ptrstpp950 commented May 28, 2013

jmarnold commented May 28, 2013

ptrstpp950 commented May 28, 2013

jmarnold commented May 28, 2013

jmarnold commented Jun 7, 2013

ptrstpp950 commented Jun 7, 2013

Scooletz commented Jun 12, 2013

jmarnold commented Jun 12, 2013

ptrstpp950 commented Jun 12, 2013

jmarnold commented Sep 18, 2013

cleaning dependencies before update nuspec #84

Are you sure you want to change the base?

cleaning dependencies before update nuspec #84

Conversation

ptrstpp950 commented May 22, 2013

jmarnold commented May 22, 2013

ptrstpp950 commented May 22, 2013

jmarnold commented May 22, 2013

ptrstpp950 commented May 22, 2013

ptrstpp950 commented May 22, 2013

jmarnold commented May 22, 2013

ptrstpp950 commented May 28, 2013

jmarnold commented May 28, 2013

ptrstpp950 commented May 28, 2013

jmarnold commented May 28, 2013

ptrstpp950 commented May 28, 2013

jmarnold commented May 28, 2013

jmarnold commented Jun 7, 2013

ptrstpp950 commented Jun 7, 2013

Scooletz commented Jun 12, 2013

jmarnold commented Jun 12, 2013

ptrstpp950 commented Jun 12, 2013

jmarnold commented Sep 18, 2013