search

desyncr / zpm-zsh

Main repository: https://github.com/zpm-project/zpm-zsh
GNU General Public License v3.0
2 stars 4 forks source link

Add save command. #25

Closed fennecdjay closed 7 years ago

fennecdjay commented 7 years ago

Here it is ;-) It looks just fine to me.

fennecdjay commented 7 years ago

We're almost done! I'd just like rmdir_r() to remove user directory if it is empty.

desyncr commented 7 years ago

Nice! I'm gonna take a look tonight!

fennecdjay commented 7 years ago

Maybe you should rework the README.md, as you clearly know zpm's workflow better than I do.

desyncr commented 7 years ago

Seems fair. I'll create another issue for the README.md

desyncr commented 7 years ago

Last thing would be to add remove and uninstall to the list of available commands.

desyncr commented 7 years ago

On a quick test it failed to uninstall the plugin directory. It seems it removed the .git directory though.

I also believe we could change terms: "remove" for unlinking/remove directory, and "disable" to remove plugin from listing. It's a small thing but it's best to get it correct the first time.

fennecdjay commented 7 years ago

If you agree, I'll comment everything in rmdir_r(), and just use something like

system("rm -r xxx")

so we get the functionnality, until I fix that function.

desyncr commented 7 years ago

@fennecdjay I do agree. For now it'll suffice.

fennecdjay commented 7 years ago

We can ignore Travis warning for now, as it complains about rake (?). On my station it seems to work just fine.

fennecdjay commented 7 years ago

Also, I see you created zpm-project organisation, so I suppose you expect next PR to be there.

fennecdjay commented 7 years ago

Oups... Pushing the rigth thing now.

desyncr commented 7 years ago

@fennecdjay There is a lot of comments and useful interactions, so let's keep it here until the final approval. Then we'll move to zpm-project. Sorry for the mess up but it has to be sooner or later :)

fennecdjay commented 7 years ago

Very good to me. ;-)

desyncr commented 7 years ago

One last thing then we can merge this. Once done you have to create a new PR in zpm-project/zpm-zsh repository.

fennecdjay commented 7 years ago

I'm waiting for your signal to either do more changes or create PR in zpm-project/zpm-zsh repository.

fennecdjay commented 7 years ago

Great! doing it rigth now.

desyncr commented 7 years ago

Also, remember to review https://github.com/zpm-project/zpm-zsh/pull/6

fennecdjay commented 7 years ago

Sure thing! Yet I have to figure out how to do so ;-)

desyncr commented 7 years ago

Moved to https://github.com/zpm-project/zpm-zsh/pull/12