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

[WIP] Why oh why? #1960

Merged
merged 11 commits into from Oct 22, 2016
Merged

[WIP] Why oh why? #1960

merged 11 commits into from Oct 22, 2016

Conversation

theimowski
Copy link
Member

@theimowski theimowski commented Oct 12, 2016

#1959

TODO:

  • recognize github dependencies
  • show dependencies up to the top level
  • option for displaying version and depending packages version constraints
  • show the shortest path by default, give option to show all paths
  • mark all direct dependencies in path
  • reword "dependency path" -> "dependency chain", make the output more user-friendly
  • what should be displayed when asking for a direct dependency, and what if it is not a top-level dependency?
  • handle circular dependencies ?

Feedback welcome!

image

@cdrnet
Copy link
Member

cdrnet commented Oct 12, 2016

This will become very useful, thanks! Could it also provide some info on why a particular version is used (or what is effective version constraints are), if applicable?

@Stift
Copy link
Contributor

Stift commented Oct 12, 2016

When a package dependency is shown, do we show all dependencies up the dependency tree up to the top-level, only top level or only to the next level?
Could also be a command switch like --full

@forki
Copy link
Member

forki commented Oct 13, 2016

@theimowski please keep posting screenshots if you change something. This is awesome

@theimowski
Copy link
Member Author

will do, I'll also let know whenever I get to the point where it could be merged - possibly with not yet all features implemented

@isaacabraham
Copy link
Contributor

Would it be worth making package id optional - so that it shows why all packages are in the lock file for a given group (although this might be a bit verbose)?

@theimowski
Copy link
Member Author

@Stift I think it makes sense to show all deps up to the top level by default.
What I'm wondering is whether it makes sense to print all paths from the top level package, or just choose a shortest one? (see below e.g. Microsoft.Bcl.Build)

image

@forki
Copy link
Member

forki commented Oct 14, 2016

Maybe show the shortest and say "and x more paths"?

@tommes
Copy link

tommes commented Oct 14, 2016

Maybe (as a 2nd step) we couln't just ask for package id's, but also for artefacts - i.e. why is the dll/js/whatever in my output. In case I've overlooked that just ignore the comment.

@forki
Copy link
Member

forki commented Oct 14, 2016

That would be brilliant

Am 14.10.2016 09:27 schrieb "Thomas Koch" notifications@github.com:

Maybe (as a 2nd step) we couln't just ask for package id's, but also for
artefacts - i.e. why is the dll/js/whatever in my output. In case I've
overlooked that just ignore the comment.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1960 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNNqWsjTTQN3r1hG2igLDMaTj3jFyks5qzy7JgaJpZM4KUnL8
.

@forki
Copy link
Member

forki commented Oct 14, 2016

@tommes but it's probably hard to decide without reimplementing MSBuild logic

@Stift
Copy link
Contributor

Stift commented Oct 14, 2016

I think by package Id would be sufficient for the most use case.
If there are more than one path then, I'd like to have an option like -full or -all, if possible.
If a tree notation would be too complex I would recommend sth like ´Package1 >> Package2 >> Package3´
@theimowski Would it make sense to merge the trees, so that the output is more compact?

@forki
Copy link
Member

forki commented Oct 14, 2016

If we merge the trees then we will get the lock file back

Am 14.10.2016 17:05 schrieb "Christian Fiebrig" notifications@github.com:

I think by package Id would be sufficient for the most use case.
If there are more than one path then, I'd like to have an option like
-full or -all, if possible.
If a tree notation would be too complex I would recommend sth like
´Package1 >> Package2 >> Package3´
@theimowski https://github.com/theimowski Would it make sense to merge
the trees, so that the output is more compact?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1960 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNKInm4QJfXA1ySiysueDfyEI7exUks5qz5otgaJpZM4KUnL8
.

@theimowski
Copy link
Member Author

Can NuGets have cyclic dependencies? And if so does it resolve to paket.lock?

@theimowski
Copy link
Member Author

How about this? I've grouped the paths by the top level dependency and if from a specific top level dep there are >1 paths, we show shortest by default.
This way we won't ignore multiple top-level deps

image

@forki forki merged commit 3714b8f into fsprojects:master Oct 22, 2016
@forki
Copy link
Member

forki commented Oct 22, 2016

I released it in alpha. please create new PRs to keep it going ;-)

@theimowski
Copy link
Member Author

Alright! Can one continue work on the same branch and reopen PR in github or do i have to create new PRs?

@forki
Copy link
Member

forki commented Oct 25, 2016

You can try to reopen. Not sure if this works.

Am 22.10.2016 17:51 schrieb "Tomasz Heimowski" notifications@github.com:

Alright! Can one continue work on the same branch and reopen PR in github
or do i have to create new PRs?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#1960 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNB4iTKN8Yg9uhKtq4re1_beW-mO2ks5q2jEAgaJpZM4KUnL8
.

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.

None yet

6 participants