leiningen icon indicating copy to clipboard operation
leiningen copied to clipboard

NOT ACTUALLY READY TO MERGE Warn on checkout/dep mismatch

Open j-po opened this issue 9 years ago • 4 comments

@hyPiRion : this is the basic skeleton of what I think it's going to look like, but I'm curious

  1. if there's a canonical way to get the filesystem path of a project (see :path-goes-here in my diff)
  2. what a good automated test strategy for a warning would be
  3. if you have any other comments.

Thanks for offering your help with this, and sorry for the delay in getting to it.

j-po avatar Mar 07 '16 04:03 j-po

@j-po: Thanks! I'll have a look at this later today.

hypirion avatar Mar 08 '16 09:03 hypirion

Also, running lein bootstrap and bin/lein compile before I started my work changed leiningen-core/pom.xml. Should this go in my PR, or is it a quirk of local development?

j-po avatar Mar 14 '16 02:03 j-po

Sorry about my rather flexible definition of "later today", I lost this one among some other things I wanted to do. It's next up on my list of things to do, which will hopefully be tomorrow.

You should probably leave leiningen-core/pom.xml out of the commits, but it's not a big deal if you've accidentally slipped it in. It was added to fix #745, but I'm not sure whether this has actually been used lately.

hypirion avatar Mar 15 '16 00:03 hypirion

Alright, to the questions you asked:

  1. You could use (.getCanonicalFile dep) or (.getCanonicalPath dep) inside read-checkouts to get the canonical path to the project root, which you can then pass to the warn function
  2. I wish there were any good automated test strategies we could use here, but as far as testing goes, I wouldn't mind just testing the presence and absence of outdated checkouts for a single project. You can probably create a new test project and refer to some existing projects in https://github.com/technomancy/leiningen/tree/master/test_projects (although I guess you'd have to place the test in leiningen, not leiningen-core, then)
  3. The approach looks good, but do you think you could attempt to limit the lines to around 80 columns? Also, I think the last part of the message ", not the released version." would probably be better read as ", not version" project-dep-ver, so that the user knows what's in the project.clj.

Thank you for being patient with me here.

hypirion avatar Mar 15 '16 22:03 hypirion