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

Improve unicode handling when listing excluded PRs #216

Merged
merged 1 commit into from
Feb 16, 2017

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Feb 3, 2017

Addresses #188 (comment)

For PR which titles contains Unicode (as the ones truncated by GitHub from long commit headers), the formatting of the exclusion PRs in the log was causing UnicodeDecodeError.

No integration tests added yet. However this could be functionally tested as follows.

From a repository with open PRs containing Unicode characters in their title, force the PR(s) to be excluded and check the current version of scc merge is throwing an error, e.g. using the current state of https://github.com/IDR/idr-metadata, the following command should fail

scc merge master --info -Euser:eleanorwilliams

With this PR included, the same command should complete and list the excluded PRs.

For PR which titles contains Unicode (as the ones truncated by GitHub from
long commit headers), the formatting of the exclusion PRs in the log was
causing UnicodeDecodeError.
@sbesson sbesson mentioned this pull request Feb 15, 2017
@sbesson sbesson modified the milestone: 0.7.0 Feb 15, 2017
@atarkowska
Copy link
Member

atarkowska commented Feb 16, 2017

without this PR

Traceback (most recent call last):
  File "/home/omero/.local/lib/python2.7/site-packages/scc/main.py", line 76, in entry_point
    (UpdateSubmodules.NAME, UpdateSubmodules),
  File "/home/omero/.local/lib/python2.7/site-packages/yaclifw/framework.py", line 197, in main
    ns.func(ns)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 2942, in __call__
    self.merge(args, self.main_repo)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 2972, in merge
    set_commit_status=args.set_commit_status)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 1637, in rmerge
    merge_msg += self.origin.find_candidate_pulls(filters)
  File "/home/omero/.local/lib/python2.7/site-packages/scc/git.py", line 850, in find_candidate_pulls
    msg += str(pull) + " (%s)" % excluded_pulls[pull] + "\n"
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 103: ordinal not in range(128)
Build step 'Execute shell' marked build as failure

with this PR works as expected


+ pip install --user -U git+git://github.com/sbesson/snoopycrimecop@excluded_unicode#egg=scc
Collecting scc from git+git://github.com/sbesson/snoopycrimecop@excluded_unicode#egg=scc
  Cloning git://github.com/sbesson/snoopycrimecop (to excluded_unicode) to /tmp/pip-build-Bt4EOA/scc
Requirement already up-to-date: yaclifw>=0.1.2 in /home/omero/.local/lib/python2.7/site-packages (from scc)
Requirement already up-to-date: PyGithub in /home/omero/.local/lib/python2.7/site-packages (from scc)
Requirement already up-to-date: argparse in /home/omero/.local/lib/python2.7/site-packages (from scc)
Requirement already up-to-date: python-jose in /home/omero/.local/lib/python2.7/site-packages (from PyGithub->scc)
Requirement already up-to-date: pycrypto<2.7.0,>=2.6.0 in /home/omero/.local/lib/python2.7/site-packages (from python-jose->PyGithub->scc)
Collecting six<2.0 (from python-jose->PyGithub->scc)
  Downloading six-1.10.0-py2.py3-none-any.whl
Requirement already up-to-date: ecdsa<1.0 in /home/omero/.local/lib/python2.7/site-packages (from python-jose->PyGithub->scc)
Requirement already up-to-date: future<1.0 in /home/omero/.local/lib/python2.7/site-packages (from python-jose->PyGithub->scc)
Installing collected packages: scc, six
  Found existing installation: scc 0.6.6-19-g14a4
    Uninstalling scc-0.6.6-19-g14a4:
      Successfully uninstalled scc-0.6.6-19-g14a4
  Running setup.py install for scc: started
    Running setup.py install for scc: finished with status 'done'
Successfully installed scc-0.6.6-15-ga1e4 six-1.10.0
+ test -e src
+ cd src
+ /home/omero/.local/bin/scc merge develop --no-ask --reset --update-gitmodules -S success-only --push develop/merge/trigger
2017-02-16 09:29:36,472 [   scc.merge] INFO  Merging Pull Request(s) based on develop
2017-02-16 09:29:36,472 [   scc.merge] INFO  Including Pull Request(s) opened by any public member of the organization
2017-02-16 09:29:36,472 [   scc.merge] INFO  Including Pull Request(s) labelled as include
2017-02-16 09:29:36,472 [   scc.merge] INFO  Excluding Pull Request(s) labelled as exclude or breaking
2017-02-16 09:29:36,472 [   scc.merge] INFO  Excluding Pull Request(s) without successful status
2017-02-16 09:32:01,527 [   scc.merge] INFO  Repository: openmicroscopy/openmicroscopy
2017-02-16 09:32:01,527 [   scc.merge] INFO  Excluded PRs:
...
2017-02-16 09:32:01,528 [   scc.merge] INFO  Already up-to-date.
2017-02-16 09:32:01,528 [   scc.merge] INFO  
2017-02-16 09:32:01,528 [   scc.merge] INFO  Merged PRs:
...
2017-02-16 09:32:01,529 [   scc.merge] INFO  Repository: ome/scripts
2017-02-16 09:32:01,529 [   scc.merge] INFO  Already up-to-date.
2017-02-16 09:32:01,529 [   scc.merge] INFO  
2017-02-16 09:32:01,529 [   scc.merge] INFO  
2017-02-16 09:32:10,824 [   scc.merge] INFO  Merged branch pushed to https://github.com/snoopycrimecop/openmicroscopy/tree/develop/merge/trigger
Finished: SUCCESS

@sbesson
Copy link
Member Author

sbesson commented Feb 16, 2017

Merging for 0.7.0

@sbesson sbesson merged commit 1a128ea into ome:master Feb 16, 2017
@sbesson sbesson deleted the excluded_unicode branch February 16, 2017 09:45
@sbesson sbesson mentioned this pull request Feb 16, 2017
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.

2 participants