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

Feature request: command to add new repository #333

Open
aschleifer opened this issue Feb 22, 2022 · 22 comments
Open

Feature request: command to add new repository #333

aschleifer opened this issue Feb 22, 2022 · 22 comments

Comments

@aschleifer
Copy link
Contributor

Currently when using this tool one has to edit the config file and then run the vcspull command (maybe with filter on the new repo name) to get the new checkout.

I would propose to add some sort of command where you can do something like vcspull add <repoUrl> which will then generate an entry similar to:

<folder>:
  <repoName>: '<repoUrl>'

in the existing config. would be the folder from which the call is executed Obviously if the already exist it should just add an entry to that block. The should be extractable from the , if not it needs to be passed in the call.

Once this config update was done vcspull should be able to run its normal "checkout/update" logic on that new entry to create it.

That way you can add a new entry in the vcspull config and get the checkout right away.

Any thoughts on this?

Best regards
Segaja

@rogerioefonseca
Copy link

I liked the idea, I'm using the tool as well to organize my private repos, and would be nice to not add manually configuration.

And that could work in the other way around as well to remove the entry, haha.

@tony
Copy link
Member

tony commented Feb 23, 2022

This sounds good to me

@tony
Copy link
Member

tony commented Feb 23, 2022

@rogerioefonseca @aschleifer

Unrelated: Do you have any objection to the behavior of the empty command (e.g. vcspull) requiring vcspull sync or vcspull s to initiate a sync? As it stands vcspull automatically syncs, but i'd like to change it to a command list

The old behavior would be possible by making an alias to vcspull sync

@aschleifer
Copy link
Contributor Author

I personally think it makes sense to have "subcommands" you need to execute. And maybe show a usage if you don't pass the subcommand.

That way we could have "sync" as the "normal sync", maybe "import" or "scan" for your mentioned idea of scanning an existing folder structure and generating a config out of it.
For the current example we could then do "add" for adding one new repo (as described in the description of this issue) and so on.

For code structure it could make sense to also split the code for these subcommands each into its own file. that way it is easier to maintain the code and "see" where what is happening.

@rogerioefonseca
Copy link

I like the idea of having a list of commands instead of starting with the principle that it will by default execute some magical command, and having that, I agree with you to add the vcspull sync

@tony
Copy link
Member

tony commented Feb 24, 2022

I will look at this above feedback and also make giving the default command list "subcommands"

@tony
Copy link
Member

tony commented Feb 24, 2022

That way we could have "sync" as the "normal sync", maybe "import" or "scan" for your mentioned idea of scanning an existing folder structure and generating a config out of it. For the current example we could then do "add" for adding one new repo (as described in the description of this issue) and so on.

Noted

For code structure it could make sense to also split the code for these subcommands each into its own file. that way it is easier to maintain the code and "see" where what is happening.

Agreed

I like the idea of having a list of commands instead of starting with the principle that it will by default execute some magical command, and having that, I agree with you to add the vcspull sync

I agree with this sentiment. Next release will move the sync behavior to vcspull sync

@aschleifer
Copy link
Contributor Author

@tony who knows, if you bring the current master codebase into the new structure (like moving the current "sync" code into its own file and proposing the structure and invocation) I might pick up the idea of this issue here and add the new command myself.

Working on the generator script in python got me enjoying python again, so if you don't mind i could implement this "add" feature myself.

@tony
Copy link
Member

tony commented Feb 25, 2022

who knows, if you bring the current master codebase into the new structure (like moving the current "sync" code into its own file and proposing the structure and invocation) I might pick up the idea of this issue here and add the new command myself.

My first order of business this weekend will be to do that

P.S. If you have the codebase checked out, do you use poetry? You can run poetry shell, poetry install on the project

If you have that setup, does make test work?

If you install entr (https://github.com/eradman/entr), you can also do make watch_test to rerun on file changes

@aschleifer
Copy link
Contributor Author

who knows, if you bring the current master codebase into the new structure (like moving the current "sync" code into its own file and proposing the structure and invocation) I might pick up the idea of this issue here and add the new command myself.

My first order of business this weekend will be to do that

Nice, I will watch for changes. Also it might be that my other account ( Segaja ) then interacts with this repository.

P.S. If you have the codebase checked out, do you use poetry? You can run poetry shell, poetry install on the project

If you have that setup, does make test work?

If you install entr (https://github.com/eradman/entr), you can also do make watch_test to rerun on file changes

Right now I have a fork of this repo where I did the changes on, I didn't use poetry yet, but if I get into feature development on this project then I guess it makes sense to get familiar with these things.

@tony
Copy link
Member

tony commented Feb 25, 2022

Noted on the other account

Also, while the development loop will be the same, in the coming weeks I may make some refactors between https://github.com/vcs-python/libvcs/ and https://github.com/vcs-python/vcspull/. I haven't taken a fresh look at the APIs in a long time

@aschleifer
Copy link
Contributor Author

P.S. If you have the codebase checked out, do you use poetry? You can run poetry shell, poetry install on the project

If you have that setup, does make test work?

If you install entr (https://github.com/eradman/entr), you can also do make watch_test to rerun on file changes

Btw, is all this documented in some markdown file in the repo for other people for local development? It might help others to jump in and contribute to this project.

@tony
Copy link
Member

tony commented Feb 25, 2022

Btw, is all this documented in some markdown file in the repo for other people for local development? It might help others to jump in and contribute to this project.

Yes - and I plan on overhauling / improving it more

markdown: https://github.com/vcs-python/vcspull/blob/master/docs/developing.md
html: https://vcspull.git-pull.com/developing.html

@tony
Copy link
Member

tony commented Mar 1, 2022

who knows, if you bring the current master codebase into the new structure (like moving the current "sync" code into its own file and proposing the structure and invocation) I might pick up the idea of this issue here and add the new command myself.

v1.11.0: vcspull/, changelog

vcspull sync now a separate command in its own file

more to come, including fixing shell completions.

It's there, but broken:

  • Bash: eval "$(_VCSPULL_COMPLETE=bash_source vcspull)"
  • ZSH: eval "$(_VCSPULL_COMPLETE=zsh_source vscpull)"

vcspull sync <tab>
vcspull sync -c <tab> <tab>

@Segaja
Copy link

Segaja commented Mar 1, 2022

Nice work. I will look into it as soon as I find time. But one thing I saw already. You are using a 3 digit version number which implies you are following Semantic Versioning, but actually you are doing breaking changes in a "feature version" (1.10.0).

In semantic versioning such change would have been released as 2.0.0 to indicate to people that something breaks.

Since you didn't choose 2.0.0 as a version I guess you didn't hear of Semantic Versioning before, which is fine. Just wanted to give you a heads up that this can lead to confusion. Also for people who want to package this for linux distributions (as I might do soon for Arch Linux).

Best regards.

@tony
Copy link
Member

tony commented Mar 1, 2022

It's true. I'm using an abbreviated version policy, I should add a note we don't follow sem ver and APIs are unstable. this doesn't have as many users as say, tmuxp and libtmux.

With semver, I'd end up having a release number that's like Firefox or Chrome's. Which this thing could adopt.

As of yet, I am going by the norms usually seen on python community. It'd look odd as a python project

As an example in the poetry package manager, 1.0 to 1.1, 1.1 to 1.2, they break majorly, in django's early releases: https://docs.djangoproject.com/en/dev/internals/deprecation/ deprecations happen between 1.4 to 1.6 for instance, their deprecation policy

For the library, I am more conservative with shifting APIs around because its a library and actually throw in deprecation warnings: https://libvcs.git-pull.com/history.html#libvcs-0-4-2020-08-01. But it has very very few users

@tony tony mentioned this issue Mar 2, 2022
3 tasks
@Segaja
Copy link

Segaja commented Mar 26, 2022

@tony in regards to developing this: Is there already a method in the code to write a loaded config back into the yaml/json config structure?

Obviously this would be needed for this command.

@tony
Copy link
Member

tony commented Mar 26, 2022

@aschleifer

@tony in regards to developing this: Is there already a method in the code to write a loaded config back into the yaml/json config structure?

Obviously this would be needed for this command.

There isn't a way to do this as of yet.

The closest thing would be a test of ours using kaptan.Kaptan.export() to write to a fixture file:

def test_export_json(tmp_path: pathlib.Path):
json_config = tmp_path / ".vcspull.json"
json_config_file = str(json_config)
config = kaptan.Kaptan()
config.import_config(fixtures.config_dict)
json_config_data = config.export("json", indent=2)
json_config.write_text(json_config_data, encoding="utf-8")
new_config = kaptan.Kaptan().import_config(json_config_file).get()
assert fixtures.config_dict == new_config

Most of my efforts over the weeks have been "untangling" to make efforts like this easier.

@Segaja
Copy link

Segaja commented Mar 26, 2022

Hm the "problem" is that load_config() is using kaptan but then calls extract_repos() which enriches the config file. to be honest the original kaptan output is much easier to work with in that regard.

For writing this back to file I guess I have to undo the work of extract_repos() and bring the config object back into the structure that kaptan understands.

@tony
Copy link
Member

tony commented Mar 26, 2022

@Segaja Part of me wishes I never added those extract_repo/enrichment things.

If it was just purely the dictionary structure, with the exception of expanduser/expandvars, the code would be far simpler.

@Segaja
Copy link

Segaja commented Mar 26, 2022

I agree. Can we rip it out? :D

@tony
Copy link
Member

tony commented Mar 26, 2022

I'm deliberating doing the same to tmuxp.

I agree. Can we rip it out? :D

I created #360 to brainstorm a config format that'd work with all purposes

@tony tony mentioned this issue Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants