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

Add cmd PkgREPL command #104

Closed
wants to merge 1 commit into from
Closed

Conversation

frankier
Copy link
Contributor

@frankier frankier commented Sep 1, 2023

This is useful because the run PkgREPL command was picking up my system mamba even though MicroMamba was in use. This is some type of a PATH issue probably, but at any rate, this approach which will reliably target the correct executable seems like a useful addition since it can be more robust.

@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #104 (71bd5ab) into main (4d48894) will decrease coverage by 0.65%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #104      +/-   ##
==========================================
- Coverage   92.82%   92.18%   -0.65%     
==========================================
  Files           8        8              
  Lines         864      870       +6     
==========================================
  Hits          802      802              
- Misses         62       68       +6     
Files Changed Coverage Δ
src/PkgREPL.jl 87.50% <0.00%> (-12.50%) ⬇️

@cjdoris
Copy link
Collaborator

cjdoris commented Sep 13, 2023

This is so similar to conda run that maybe they should be merged? That is, change conda run so that if the first arg is conda (or mamba or micromamba?) then run whatever CondaPkg runs?

@frankier
Copy link
Contributor Author

That makes sense. So to confirm the plan would be:

  • Do the same as conda cmd procedure when any of these match
  • Don't do it when they are quoted
  • Document this in the docstring of conda run

I'll modify this PR with these changes soonish

@frankier
Copy link
Contributor Author

Done

@cjdoris
Copy link
Collaborator

cjdoris commented Oct 21, 2023

Thanks for the PR - I came to review it and had some comments, but in the end decided it would be quickest to just implement my ideas directly. I went with a simpler version, which just checks if the first arg is "conda" regardless of quoting. I didn't think the extra complexity was worth it for what is really just a convenience function. It's released now.

@cjdoris cjdoris closed this Oct 21, 2023
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.

2 participants