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

NOT ACTUALLY READY TO MERGE Warn on checkout/dep mismatch #2107

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

j-po
Copy link

@j-po j-po commented Mar 7, 2016

@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 j-po changed the title Warn on checkout/dep mismatch NOT ACTUALLY READY TO MERGE Warn on checkout/dep mismatch Mar 7, 2016
@hypirion hypirion self-assigned this Mar 8, 2016
@hypirion
Copy link
Collaborator

hypirion commented Mar 8, 2016

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

@j-po
Copy link
Author

j-po commented Mar 14, 2016

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?

@hypirion hypirion added this to the 2.6.2 milestone Mar 14, 2016
@hypirion
Copy link
Collaborator

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
Copy link
Collaborator

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 hypirion modified the milestones: 2.6.2, 2.7.1 Aug 30, 2016
@technomancy technomancy removed this from the 2.8.0 milestone Jun 15, 2017
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.

3 participants