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 ls option to cli #599

Closed
wants to merge 0 commits into from
Closed

Add ls option to cli #599

wants to merge 0 commits into from

Conversation

pythops
Copy link
Contributor

@pythops pythops commented Jun 21, 2020

No description provided.

@tony
Copy link
Member

tony commented Jun 24, 2020

Nice @pythops ! (Also nice username!)

Can you add a test? (e.g. https://github.com/tmux-python/tmuxp/blob/master/tests/test_cli.py)?

@pythops
Copy link
Contributor Author

pythops commented Jun 27, 2020

Thanks @tony :)
Always a pleasure to contribute.

@tony
Copy link
Member

tony commented Jun 28, 2020

@pythops I will take a look at this tomorrow

I need to take a look at why Travis isn't triggering.

@tony
Copy link
Member

tony commented Jun 28, 2020

@tony
Copy link
Member

tony commented Jun 28, 2020

I will take a peek tomorrow. It looks like there's another error happening at the same time not related to this PR.

Copy link
Member

@tony tony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Some change requests

Additionally, can you do a rebase against master so the latest fixes for travis CI are in?

I'm not sure what your repo layout is like (git remotes) but it's something like this:

git remote -v

Assuming your origin is:

origin  https://github.com/tmux-python/tmuxp.git (fetch)
origin  https://github.com/tmux-python/tmuxp.git (push)

git pull --rebase origin master
git push --force-with-lease

If origin is your own branch, git remote add upstream https://github.com/tmux-python/tmuxp.git and do this:

git pull --rebase upstream master
git push --force-with-lease

tests/test_cli.py Outdated Show resolved Hide resolved
tmuxp/cli.py Outdated Show resolved Hide resolved
tmuxp/cli.py Outdated Show resolved Hide resolved
tmuxp/cli.py Outdated Show resolved Hide resolved
@pythops pythops requested a review from tony June 30, 2020 21:49
@pythops
Copy link
Contributor Author

pythops commented Jul 6, 2020

@tony Would you mind restart the tests, they should pass.
Thanks

@tony
Copy link
Member

tony commented Jul 6, 2020

@pythops Restarted

@tony
Copy link
Member

tony commented Jul 6, 2020

@pythops You can also rebase again - I did several fixes to travis since you last pushed

@pythops
Copy link
Contributor Author

pythops commented Jul 7, 2020

The CI failed because of poetry. Maybe it should be restarted again as I did the rebase after.

@tony
Copy link
Member

tony commented Jul 9, 2020

@pythops

You need to rebase the against my master branch - which has poetry - and force push it. It's not easy to give instructions on since I don't know how you have git remote names configured.

Side note: It's really difficult to find a tutorial online that's satisfactory for me to explain - since it really hinges on your local github layout and how you have git remote set. Sorry about that!

When you rebase your branch against my master branch at tmux-python/tmuxp, it will rewrite your commits on top of the latest code.

Can you give me access to your forked repo? I can do it for you

@tony
Copy link
Member

tony commented Jul 9, 2020

@pythops Hm, try this:

git remote add pythops https://github.com/tmux-python/tmuxp.git
git fetch pythops
git checkout -b add-ls-option
git reset --hard pythops/master
git push -u pythops add-ls-option

This assures that:

  1. you are using pythops as a remote for your own repo, so we're referring to the same git repo.
  2. you aren't using master as a branch for a change, since those are better kept in their own specific branches, e.g. add-ls-option
  3. the branch add-ls-option is set-upstream to push to your repo at add-ls-option in future commits

You should see an option to open a new pull request, go ahead and do it if you'd like

Now, to rebase so your work is based on the latest tmuxp:

git remote add tmux-python https://github.com/tmux-python/tmuxp.git
git fetch tmux-python
git pull --rebase tmux-python master --autostash
git push --force-with-lease

@pythops
Copy link
Contributor Author

pythops commented Jul 9, 2020

@tony you should have access to my fork

@tony tony closed this Jul 9, 2020
@tony
Copy link
Member

tony commented Jul 9, 2020

image

@tony
Copy link
Member

tony commented Jul 9, 2020

Sorry about that, not sure why the branch closed

@tony tony mentioned this pull request Jul 9, 2020
@tony
Copy link
Member

tony commented Jul 9, 2020

@pythops In what sense do I have access to your repo? I don't seem to be able to push branches

See #616, I PR'd it, but the problem is - I'd sort of take credit for your work.

I want to make sure you get credit for the Pull request/commits. I think you need to redo it and rebase correctly - it's really up to your system and reading up on rebasing, I wrote more here: #599 (comment)

Do you want to make your own pull request? If so, I think you need to figure out the branching and make a new PR. If I do it for you, I end up becoming the creator of the PR and part of the commit messages.

@tony
Copy link
Member

tony commented Jul 9, 2020

Also sorry for all the trouble with this PR!

Initially our CI system was broke. And rebasing it kind of tricky to explain without me knowing your configuration. If I gave instructions and the remote names didn't match, the instructions wouldn't work.

tony added a commit that referenced this pull request Jul 26, 2020
Almost missed this! Thank you for #599
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.

None yet

2 participants