asdf icon indicating copy to clipboard operation
asdf copied to clipboard

feat: unset all enviroment variables ASDF_{PLUGIN}_VERSION

Open edvardsanta opened this issue 2 years ago • 13 comments

it is a feature create by recommendation of issue 1619

Summary

It unset all enviroment variables ASDF_{PLUGIN}_VERSION image fish

Closes: #1619

Other Information

I was not able to test in elvish. Because of that it just prints a description

edvardsanta avatar Sep 06 '23 19:09 edvardsanta

LGTM - CI maybe needs to be ran by a maintainer though (since #1624 was merged).

hyperupcall avatar Sep 12 '23 16:09 hyperupcall

When adding new flags we would like to get some test cases for both implemented code paths, even if just the happy paths.

Looks good otherwise 💯

Great. I will try to create some tests, from what I had seen, there are no tests related to the Shell command yet, i will try to cover my feature and previous features =].

edvardsanta avatar Sep 15 '23 03:09 edvardsanta

@hyperupcall and/or @jthegedus, how can i source asdf file? I suppose to source to integrate with shell. image

edvardsanta avatar Sep 15 '23 16:09 edvardsanta

@edvardsanta Information about shell initialization on our documentation website. It could be that you are getting that error because you forgot to run asdf's initialization before running your tests. If you are getting errors from only that particular test, look at other tests to see how it's done. Please also look at the asdf code that prints that error: Shell integration is not enabled.... Those are just some ideas to figure out what your issue is - I haven't ran across that myself.

hyperupcall avatar Sep 15 '23 17:09 hyperupcall

@edvardsanta I did already review and approve your code - now you only need to update your code as per jthegedus's feedback.

hyperupcall avatar Sep 15 '23 17:09 hyperupcall

@edvardsanta Information about shell initialization on our documentation website. It could be that you are getting that error because you forgot to run asdf's initialization before running your tests. If you are getting errors from only that particular test, look at other tests to see how it's done. Please also look at the asdf code that prints that error: Shell integration is not enabled.... Those are just some ideas to figure out what your issue is - I haven't ran across that myself.

This specific test that i am creating is not working, but i will read the doc

edvardsanta avatar Sep 15 '23 17:09 edvardsanta

This specific test that i am creating is not working, but i will read the doc

Feel free to commit the WIP tests and get the CI to execute them. The tests need some work to separate out some of the shell-specific tests which might be difficult to run locally.

jthegedus avatar Sep 17 '23 11:09 jthegedus

image same problem here, i might see where is this log and do some changes again

Edit: I created the tests, sorry for too many commits, I thought there was no test for the shell command, but I found it, so I just created my test there

edvardsanta avatar Sep 21 '23 13:09 edvardsanta

I think i finalized the feature, let me know if i might change something [=.

edvardsanta avatar Sep 22 '23 19:09 edvardsanta

@jthegedus can you check my pr? pls

edvardsanta avatar Oct 04 '23 20:10 edvardsanta

@jthegedus, any updates?

edvardsanta avatar Dec 04 '23 16:12 edvardsanta

@edvardsanta Have you seen the PR backlog? I'm sure the maintainers are aware of it and will get to it (including this PR) when they can get to it. Sending multiple pings isn't likely going to do anything, and if it were me you were pinging, it would actually encourage me to ignore the PR.

hyperupcall avatar Dec 05 '23 12:12 hyperupcall

@edvardsanta Have you seen the PR backlog? I'm sure the maintainers are aware of it and will get to it (including this PR) when they can get to it. Sending multiple pings isn't likely going to do anything, and if it were me you were pinging, it would actually encourage me to ignore the PR.

Well, this has been active for a few months now, it's easier and better for me to send a message and ask than to guess, since my last message was a few months ago. Feel free to recommend ignoring this pr.

edvardsanta avatar Dec 05 '23 12:12 edvardsanta