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

app-misc/pip3line: Add new package #10336

Closed
wants to merge 1 commit into from
Closed

app-misc/pip3line: Add new package #10336

wants to merge 1 commit into from

Conversation

metrodango
Copy link
Contributor

New package

Signed-off-by: Gabriel Caudrelier gabriel.caudrelier@gmail.com
Package-Manager: Portage-2.3.49, Repoman-2.3.11

@gentoo-bot
Copy link

Copyright policy change

Please note that on 2018-09-15 Trustees have approved new Gentoo copyright policy. All contributions made to Gentoo need to follow this policy. If you include the Signed-off-by line in your commit message, you indicate that you have read the policy and agree to its terms. For more detailed explanation, please see the new Gentoo copyright policy explained article.

Pull Request assignment

Areas affected: ebuilds
Packages affected: app-misc/pip3line

app-misc/pip3line: @gentoo/proxy-maint (new package)

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). labels Nov 4, 2018
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Could you please keep your changes in a single PR? By closing an re-opening a new PR, we lost history from #10301

Also, please run repoman -dx full before pushing. It outputs many formatting error that need to be fixed.

app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
@metrodango
Copy link
Contributor Author

I was also told in the previous PR to squash all the commits into one, this has the effect of closing any PR requests associated with it, and the need to resubmit a new one.

If you have some git magic to prevent that, please share.

@ghost
Copy link

ghost commented Nov 4, 2018

Pushing your rebased branch to metrodango:master automatically closes the old PR? I have troubles believing that. I routinely rebase/squash branches for PRs and then push -f into the PR's base branch and github detects the situation and updates the PR accordingly.

@metrodango
Copy link
Contributor Author

metrodango commented Nov 4, 2018

Nothing in your answer helps me solve the issue.

I am currently doing this (googled):
git rebase -i [commit]
git push -f origin HEAD^:master
git push

if you have a better solution, i would be happy to use it.

@ghost
Copy link

ghost commented Nov 4, 2018

I'm puzzled about your 2nd line. It would appear to push force the parent commit, which could indeed be seen by github as cancelling your PR since you force push zero commits. What about replacing those two last lines with git push -f origin HEAD:master?

@metrodango
Copy link
Contributor Author

metrodango commented Nov 4, 2018

Actually just found it

git rebase -i HEAD~3
git push origin master --force

@ghost
Copy link

ghost commented Nov 4, 2018

So please, next time, refine your google-fu before writing stuff like "this has the effect of closing any PR requests associated with it". Your lack of git knowledge did. Writing stuff like this make your reviewers look like they ask unreasonable things.

@metrodango
Copy link
Contributor Author

To be fair I did not know there was a better solution before you told me about it. My google-fu has not much to do with it, I am just not all knowing.

In any case, this won't happen again, now that I know, so let's move on.

@metrodango
Copy link
Contributor Author

For the repoman formatting issues, if I remove the leading spaces the indentation gets broken.

I was told in the previous PR to re-indent the ebuild ... so now I have to choose between complying to repoman or complying to PR comments.

which one should I choose ?

@ghost
Copy link

ghost commented Nov 4, 2018

repoman expects tabs. as a general rule, when you're in doubt, look at existing ebuilds in the tree. There are tons of example of ebuilds for which repoman doesn't complain and that have proper indentation. See how they do.

@ghost ghost mentioned this pull request Nov 5, 2018
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-9999.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/metadata.xml Outdated Show resolved Hide resolved
@metrodango
Copy link
Contributor Author

I have uploaded a new version, hopefully this one will satisfy everybody.

I don't have a solution for the multiple python_targets_3_* though.

app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.4.2.ebuild Outdated Show resolved Hide resolved
@metrodango
Copy link
Contributor Author

Re-enabling distorm
Upgrading version

@metrodango
Copy link
Contributor Author

Version bump.

@Zlogene , @hsoft , do you have any more objections or changes requests ?

@metrodango
Copy link
Contributor Author

@Zlogene , @hsoft , any updates ?

@ghost
Copy link

ghost commented Nov 14, 2018

I've been rather busy in the last two weeks and I haven't done proxy-maint work. My review here was by chance. I'll be able to dedicate time to proxy-maint again soon, but there are other PR's that have more priority. If you look at our backlog, you'll see that some people have been waiting a bit longer than 10 days for merging...

But we'll get to it. It seems that we got past the biggest roadblocks.

Oh, by the way: some gentoo developers use github as a workflow tool, but some (like me) don't. Our official workflow tool is our bugzilla instance. By not opening a bug on bugzilla to accompany your PR, you limit your pool of reviewer to those who use github as a workflow tool. If I hadn't stumbled on your PR by chance, I wouldn't have seen it ever.

@metrodango
Copy link
Contributor Author

No problem, thanks for the answer.

I did open a bug though, the assignee is the one that told me to open a PR. I was not sure how to link it.

https://bugs.gentoo.org/667984

I will be waiting then.

@ghost
Copy link

ghost commented Nov 14, 2018

Oh, good. You can link them by adding this line in your commit message:

Bug: https://bugs.gentoo.org/667984

@metrodango
Copy link
Contributor Author

Yup, just did.

app-misc/pip3line/pip3line-3.5.4.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.5.4.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.5.4.ebuild Outdated Show resolved Hide resolved
app-misc/pip3line/pip3line-3.5.4.ebuild Outdated Show resolved Hide resolved
Signed-off-by: Gabriel Caudrelier gabriel.caudrelier@gmail.com
Package-Manager: Portage-2.3.51, Repoman-2.3.11
Bug: https://bugs.gentoo.org/667984
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-11-23 19:39 UTC
Newest commit scanned: 5524889
Status: ✅ good

No issues found

@ghost
Copy link

ghost commented Nov 29, 2018

Looks good, builds fine. I haven't ran it, I trust you @metrodango for this part. Merging, welcome to the proxied maintainers team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
4 participants