technomancy / leiningen

Moved to Codeberg; this is a convenience mirror
https://codeberg.org/leiningen/leiningen
Other
7.29k stars 1.61k forks source link

print newest deps #2792

Open cdzwm opened 2 years ago

cdzwm commented 2 years ago

When conflict deps found, print the depency of newest version.

technomancy commented 2 years ago

Thanks for this patch.

I think overall it's good to surface this information, but the way it's presented in this implementation is not particularly clear. Simply saying Suggest: ... is confusing because it's presented alongside the suggestion to use :exclusions and it's not obvious how the two different suggestions are related. I think this needs some more thought put into it, but we could include it if the information is presented in a less confusing way.

There's also a couple technical issues with it; for one it removes :eval-in :leiningen which is pretty important, so that needs to be changed.

Also it appears to bring in a new dependency, which causes a lot of extra work for downstream packagers. As far as I can tell, this is only being used for a single function, which duplicates logic already available from Aether: https://github.com/technomancy/leiningen/blob/master/leiningen-core/src/leiningen/core/pedantic.clj#L68 Let's use the existing implementation instead of bringing in something redundant.

cdzwm commented 2 years ago

According to you suggestion, I make some modifications.

technomancy commented 2 years ago

Sorry, I haven't gotten a chance to look at this yet, but I don't think it needs to be closed. Was that intentional?

cdzwm commented 2 years ago

II'd like to think about this modification again. I think there are still some problems, but you can also take a look at it first and suggest some changes.

cdzwm commented 2 years ago

I will make some fixes.

cdzwm commented 2 years ago

Fix is completed.