fixed pack package dependencies for dependent projects #1417

Merged
merged 3 commits into from Jan 21, 2016

Conversation

Projects
None yet
2 participants
@pms1969
Contributor

pms1969 commented Jan 20, 2016

changed the doco around include-referenced-projects
fixed dependency resolution for project dependencies

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 20, 2016

Member

does it still exclude stuff that has it's own template file?

Member

forki commented Jan 20, 2016

does it still exclude stuff that has it's own template file?

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 20, 2016

Contributor

no. I'll add that in now.

Contributor

pms1969 commented Jan 20, 2016

no. I'll add that in now.

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 20, 2016

Contributor

Just noticed a problem with ProjectFile.findCorrespondingFile

I think the indentation for findInDir is all wrong, and as a result, it never does that bit of searching.

I can fix that, but is the purpose of that function to find the paket.template and paket.references files for the project? If so, should it actually be tree walking up the directory stack to find them? I would imagine that they should be present in the same directory only to be considered the associated file.

Contributor

pms1969 commented Jan 20, 2016

Just noticed a problem with ProjectFile.findCorrespondingFile

I think the indentation for findInDir is all wrong, and as a result, it never does that bit of searching.

I can fix that, but is the purpose of that function to find the paket.template and paket.references files for the project? If so, should it actually be tree walking up the directory stack to find them? I would imagine that they should be present in the same directory only to be considered the associated file.

Paul Saunders
removed any pakages that have project templates
ensured this cascaded recursively
@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 20, 2016

Contributor

Working in the following way.

  • Projects are recursively traversed.
  • Any projects with their own paket.template are added as dependencies
  • Any other projects have their list of dependencies added.

I'd like to do a bit more testing before hitting go on this, but need to be able to sync up with my laptop.

Contributor

pms1969 commented Jan 20, 2016

Working in the following way.

  • Projects are recursively traversed.
  • Any projects with their own paket.template are added as dependencies
  • Any other projects have their list of dependencies added.

I'd like to do a bit more testing before hitting go on this, but need to be able to sync up with my laptop.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 21, 2016

Member

cool. let me know when you are ready

Member

forki commented Jan 21, 2016

cool. let me know when you are ready

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 21, 2016

Contributor

should be ready.

You may want to consider putting this into the 3.0 branch, as there's a breaking change
that change being that dependent project dll's are not added to the package now unless the switch included-dependent-projects is set.

What it does now do tho, is walk the project dependency tree recursively if that switch is set. It does it for dependencies, sources and dll's, which imho is an improvement of nuget, as it only goes one level down.

Contributor

pms1969 commented Jan 21, 2016

should be ready.

You may want to consider putting this into the 3.0 branch, as there's a breaking change
that change being that dependent project dll's are not added to the package now unless the switch included-dependent-projects is set.

What it does now do tho, is walk the project dependency tree recursively if that switch is set. It does it for dependencies, sources and dll's, which imho is an improvement of nuget, as it only goes one level down.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 21, 2016

Member

that dependent project dll's are not added to the package now unless the switch included-dependent-projects is set

but we never wanted to do this anyway, right? I mean we epected that this did not happen. Or am I wrong here?

Member

forki commented Jan 21, 2016

that dependent project dll's are not added to the package now unless the switch included-dependent-projects is set

but we never wanted to do this anyway, right? I mean we epected that this did not happen. Or am I wrong here?

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 21, 2016

Contributor

Direct project dlls had already been added, so my previous commit didn't change that behaviour. But you are correct, the behaviour in that commit is what we want.

Contributor

pms1969 commented Jan 21, 2016

Direct project dlls had already been added, so my previous commit didn't change that behaviour. But you are correct, the behaviour in that commit is what we want.

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 21, 2016

Contributor

Sorry, this commit, is the correct behaviour.

Contributor

pms1969 commented Jan 21, 2016

Sorry, this commit, is the correct behaviour.

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 21, 2016

Contributor

No edit on mobile. :-(

Contributor

pms1969 commented Jan 21, 2016

No edit on mobile. :-(

@forki forki merged commit bee4231 into fsprojects:master Jan 21, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 21, 2016

Member

thx

Member

forki commented Jan 21, 2016

thx

@pms1969 pms1969 deleted the pms1969:cascade-paket-dependencies branch Jan 21, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 24, 2016

Member

can youn please send a PR for docs update? Thx

Member

forki commented Jan 24, 2016

can youn please send a PR for docs update? Thx

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 24, 2016

Contributor

the original commit had some document changes. Tho I'll add some extra wording to make sure it's clear that it's recursive, and that projects with their own project template files are ignored.

Contributor

pms1969 commented Jan 24, 2016

the original commit had some document changes. Tho I'll add some extra wording to make sure it's clear that it's recursive, and that projects with their own project template files are ignored.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jan 24, 2016

Member

Do we have the parameter documented? Where exactly?
On Jan 24, 2016 11:51 PM, "Paul Saunders" notifications@github.com wrote:

the original commit had some document changes. Tho I'll add some extra
wording to make sure it's clear that it's recursive, and that projects with
their own project template files are ignored.


Reply to this email directly or view it on GitHub
#1417 (comment).

Member

forki commented Jan 24, 2016

Do we have the parameter documented? Where exactly?
On Jan 24, 2016 11:51 PM, "Paul Saunders" notifications@github.com wrote:

the original commit had some document changes. Tho I'll add some extra
wording to make sure it's clear that it's recursive, and that projects with
their own project template files are ignored.


Reply to this email directly or view it on GitHub
#1417 (comment).

@pms1969

This comment has been minimized.

Show comment
Hide comment
Contributor

pms1969 commented Jan 24, 2016

@pms1969

This comment has been minimized.

Show comment
Hide comment
@pms1969

pms1969 Jan 24, 2016

Contributor

hmm, can't find that in the solution any more.

Contributor

pms1969 commented Jan 24, 2016

hmm, can't find that in the solution any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment