You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/13 12:51:28 UTC
[GitHub] [arrow] kszucs opened a new pull request #10012: ARROW-12360: [Dev] Unify source code formatting of Archery
kszucs opened a new pull request #10012:
URL: https://github.com/apache/arrow/pull/10012
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] xhochy commented on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
xhochy commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-819513800
(I am in favor of using black more generally, so no objections to use it within the archery code ;))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-848651080
I'm not insisting on this then :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on a change in pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs commented on a change in pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#discussion_r616610529
##########
File path: dev/archery/archery/bot.py
##########
@@ -187,46 +189,60 @@ def _clone_arrow_and_crossbow(dest, crossbow_repo, pull_request):
Object containing information about the pull request the comment bot
was triggered from.
"""
- arrow_path = dest / 'arrow'
- queue_path = dest / 'crossbow'
+ arrow_path = dest / "arrow"
+ queue_path = dest / "crossbow"
# clone arrow and checkout the pull request's branch
- pull_request_ref = 'pull/{}/head:{}'.format(
+ pull_request_ref = "pull/{}/head:{}".format(
pull_request.number, pull_request.head.ref
)
git.clone(pull_request.base.repo.clone_url, str(arrow_path))
- git.fetch('origin', pull_request_ref, git_dir=arrow_path)
+ git.fetch("origin", pull_request_ref, git_dir=arrow_path)
git.checkout(pull_request.head.ref, git_dir=arrow_path)
# clone crossbow repository
- crossbow_url = 'https://github.com/{}'.format(crossbow_repo)
+ crossbow_url = f"https://github.com/{crossbow_repo}"
git.clone(crossbow_url, str(queue_path))
# initialize crossbow objects
- github_token = os.environ['CROSSBOW_GITHUB_TOKEN']
+ github_token = os.environ["CROSSBOW_GITHUB_TOKEN"]
arrow = Repo(arrow_path)
queue = Queue(queue_path, github_token=github_token, require_https=True)
return (arrow, queue)
@crossbow.command()
-@click.argument('tasks', nargs=-1, required=False)
-@click.option('--group', '-g', 'groups', multiple=True,
- help='Submit task groups as defined in tests.yml')
-@click.option('--param', '-p', 'params', multiple=True,
- help='Additional task parameters for rendering the CI templates')
-@click.option('--arrow-version', '-v', default=None,
- help='Set target version explicitly.')
+@click.argument("tasks", nargs=-1, required=False)
+@click.option(
+ "--group",
+ "-g",
+ "groups",
+ multiple=True,
+ help="Submit task groups as defined in tests.yml",
+)
+@click.option(
+ "--param",
+ "-p",
+ "params",
+ multiple=True,
+ help="Additional task parameters for rendering the CI templates",
+)
+@click.option(
+ "--arrow-version",
+ "-v",
+ default=None,
+ help="Set target version explicitly.",
+)
Review comment:
I don't like that either, but I can tolerate that in exchange of automatic formatting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #10012: ARROW-12360: [Dev] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-818820050
Perhaps ask others as well @jorisvandenbossche @xhochy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10012: ARROW-12360: [Dev] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-818815499
No problem with me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#discussion_r616646503
##########
File path: dev/archery/archery/bot.py
##########
@@ -187,46 +189,60 @@ def _clone_arrow_and_crossbow(dest, crossbow_repo, pull_request):
Object containing information about the pull request the comment bot
was triggered from.
"""
- arrow_path = dest / 'arrow'
- queue_path = dest / 'crossbow'
+ arrow_path = dest / "arrow"
+ queue_path = dest / "crossbow"
# clone arrow and checkout the pull request's branch
- pull_request_ref = 'pull/{}/head:{}'.format(
+ pull_request_ref = "pull/{}/head:{}".format(
pull_request.number, pull_request.head.ref
)
git.clone(pull_request.base.repo.clone_url, str(arrow_path))
- git.fetch('origin', pull_request_ref, git_dir=arrow_path)
+ git.fetch("origin", pull_request_ref, git_dir=arrow_path)
git.checkout(pull_request.head.ref, git_dir=arrow_path)
# clone crossbow repository
- crossbow_url = 'https://github.com/{}'.format(crossbow_repo)
+ crossbow_url = f"https://github.com/{crossbow_repo}"
git.clone(crossbow_url, str(queue_path))
# initialize crossbow objects
- github_token = os.environ['CROSSBOW_GITHUB_TOKEN']
+ github_token = os.environ["CROSSBOW_GITHUB_TOKEN"]
arrow = Repo(arrow_path)
queue = Queue(queue_path, github_token=github_token, require_https=True)
return (arrow, queue)
@crossbow.command()
-@click.argument('tasks', nargs=-1, required=False)
-@click.option('--group', '-g', 'groups', multiple=True,
- help='Submit task groups as defined in tests.yml')
-@click.option('--param', '-p', 'params', multiple=True,
- help='Additional task parameters for rendering the CI templates')
-@click.option('--arrow-version', '-v', default=None,
- help='Set target version explicitly.')
+@click.argument("tasks", nargs=-1, required=False)
+@click.option(
+ "--group",
+ "-g",
+ "groups",
+ multiple=True,
+ help="Submit task groups as defined in tests.yml",
+)
+@click.option(
+ "--param",
+ "-p",
+ "params",
+ multiple=True,
+ help="Additional task parameters for rendering the CI templates",
+)
+@click.option(
+ "--arrow-version",
+ "-v",
+ default=None,
+ help="Set target version explicitly.",
+)
Review comment:
Right. The rest of our Python codebase uses autopep8, so you may want to try it too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#discussion_r614258790
##########
File path: dev/archery/archery/bot.py
##########
@@ -187,46 +189,60 @@ def _clone_arrow_and_crossbow(dest, crossbow_repo, pull_request):
Object containing information about the pull request the comment bot
was triggered from.
"""
- arrow_path = dest / 'arrow'
- queue_path = dest / 'crossbow'
+ arrow_path = dest / "arrow"
+ queue_path = dest / "crossbow"
# clone arrow and checkout the pull request's branch
- pull_request_ref = 'pull/{}/head:{}'.format(
+ pull_request_ref = "pull/{}/head:{}".format(
pull_request.number, pull_request.head.ref
)
git.clone(pull_request.base.repo.clone_url, str(arrow_path))
- git.fetch('origin', pull_request_ref, git_dir=arrow_path)
+ git.fetch("origin", pull_request_ref, git_dir=arrow_path)
git.checkout(pull_request.head.ref, git_dir=arrow_path)
# clone crossbow repository
- crossbow_url = 'https://github.com/{}'.format(crossbow_repo)
+ crossbow_url = f"https://github.com/{crossbow_repo}"
git.clone(crossbow_url, str(queue_path))
# initialize crossbow objects
- github_token = os.environ['CROSSBOW_GITHUB_TOKEN']
+ github_token = os.environ["CROSSBOW_GITHUB_TOKEN"]
arrow = Repo(arrow_path)
queue = Queue(queue_path, github_token=github_token, require_https=True)
return (arrow, queue)
@crossbow.command()
-@click.argument('tasks', nargs=-1, required=False)
-@click.option('--group', '-g', 'groups', multiple=True,
- help='Submit task groups as defined in tests.yml')
-@click.option('--param', '-p', 'params', multiple=True,
- help='Additional task parameters for rendering the CI templates')
-@click.option('--arrow-version', '-v', default=None,
- help='Set target version explicitly.')
+@click.argument("tasks", nargs=-1, required=False)
+@click.option(
+ "--group",
+ "-g",
+ "groups",
+ multiple=True,
+ help="Submit task groups as defined in tests.yml",
+)
+@click.option(
+ "--param",
+ "-p",
+ "params",
+ multiple=True,
+ help="Additional task parameters for rendering the CI templates",
+)
+@click.option(
+ "--arrow-version",
+ "-v",
+ default=None,
+ help="Set target version explicitly.",
+)
Review comment:
You can see here that `black` adds a lot of unwanted vertical size...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #10012: ARROW-12360: [Dev] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-818813133
@pitrou I added a `archery lint --archery` command to run `isort`, `black` and `flake8`. I'd prefer black for archery since it has a stricter ruleset than autopep8 has.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10012: ARROW-12360: [Dev] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-818765207
`archery lint --python` uses a different set of checks. It would be nicer IMHO if Archery followed the same rules as the rest of Python code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-848620216
Well, as I said I'd rather see `autopep8` used like in the rest of the Python code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-818928507
https://issues.apache.org/jira/browse/ARROW-12360
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-846993099
@pitrou any objections here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs edited a comment on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs edited a comment on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-846993099
@pitrou any objections here? It's fine if there is, but then I'm just going to close instead of keep rebasing :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs closed pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs closed pull request #10012:
URL: https://github.com/apache/arrow/pull/10012
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#issuecomment-818851922
(I am in favor of using black more generally, so no objections to use it within the archery code ;))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on a change in pull request #10012: ARROW-12360: [Archery] Unify source code formatting of Archery
Posted by GitBox <gi...@apache.org>.
kszucs commented on a change in pull request #10012:
URL: https://github.com/apache/arrow/pull/10012#discussion_r616812525
##########
File path: dev/archery/archery/bot.py
##########
@@ -187,46 +189,60 @@ def _clone_arrow_and_crossbow(dest, crossbow_repo, pull_request):
Object containing information about the pull request the comment bot
was triggered from.
"""
- arrow_path = dest / 'arrow'
- queue_path = dest / 'crossbow'
+ arrow_path = dest / "arrow"
+ queue_path = dest / "crossbow"
# clone arrow and checkout the pull request's branch
- pull_request_ref = 'pull/{}/head:{}'.format(
+ pull_request_ref = "pull/{}/head:{}".format(
pull_request.number, pull_request.head.ref
)
git.clone(pull_request.base.repo.clone_url, str(arrow_path))
- git.fetch('origin', pull_request_ref, git_dir=arrow_path)
+ git.fetch("origin", pull_request_ref, git_dir=arrow_path)
git.checkout(pull_request.head.ref, git_dir=arrow_path)
# clone crossbow repository
- crossbow_url = 'https://github.com/{}'.format(crossbow_repo)
+ crossbow_url = f"https://github.com/{crossbow_repo}"
git.clone(crossbow_url, str(queue_path))
# initialize crossbow objects
- github_token = os.environ['CROSSBOW_GITHUB_TOKEN']
+ github_token = os.environ["CROSSBOW_GITHUB_TOKEN"]
arrow = Repo(arrow_path)
queue = Queue(queue_path, github_token=github_token, require_https=True)
return (arrow, queue)
@crossbow.command()
-@click.argument('tasks', nargs=-1, required=False)
-@click.option('--group', '-g', 'groups', multiple=True,
- help='Submit task groups as defined in tests.yml')
-@click.option('--param', '-p', 'params', multiple=True,
- help='Additional task parameters for rendering the CI templates')
-@click.option('--arrow-version', '-v', default=None,
- help='Set target version explicitly.')
+@click.argument("tasks", nargs=-1, required=False)
+@click.option(
+ "--group",
+ "-g",
+ "groups",
+ multiple=True,
+ help="Submit task groups as defined in tests.yml",
+)
+@click.option(
+ "--param",
+ "-p",
+ "params",
+ multiple=True,
+ help="Additional task parameters for rendering the CI templates",
+)
+@click.option(
+ "--arrow-version",
+ "-v",
+ default=None,
+ help="Set target version explicitly.",
+)
Review comment:
Running autopep8 has changed 3 lines in double aggressive mode.
The output of `black` seems more homogeneous to me and it is quicker as well.
I'd still prefer the more stricter black over autopep8, but I don't insist to it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org