search

xmunoz / bugzilla2gitlab

An issue migrator
MIT License
32 stars 19 forks source link

Added support for bugzilla login #5

Closed skirill closed 6 years ago

skirill commented 6 years ago
xmunoz commented 6 years ago

Hey, thanks for the PR. I'll add my notes inline.

skirill commented 6 years ago

I've just ran flake8 bugzilla2gitlab in my working copy. First time it complained about unrecognized import-order-style directive found in setup.cfg so I've added a flake8 plugin with sudo pip install flake8-import-order after that I've noticed a number of info and warning messages:

bugzilla2gitlab/config.py:2:1: I100 Import statements are in the wrong order. import os should be before import yaml
bugzilla2gitlab/config.py:2:1: I201 Missing newline before sections or imports.
bugzilla2gitlab/config.py:3:1: I100 Import statements are in the wrong order. from collections should be before import os
bugzilla2gitlab/migrator.py:2:1: I100 Import statements are in the wrong order. from .models should be before from .utils
bugzilla2gitlab/migrator.py:3:1: I100 Import statements are in the wrong order. from .config should be before from .models
bugzilla2gitlab/models.py:3:1: I101 Imported names are in the wrong order. Should be _perform_request, format_datetime, markdown_table_row
bugzilla2gitlab/models.py:216:50: E128 continuation line under-indented for visual indent
bugzilla2gitlab/utils.py:2:1: I100 Import statements are in the wrong order. from getpass should be before import requests
bugzilla2gitlab/utils.py:2:1: I201 Missing newline before sections or imports.
bugzilla2gitlab/utils.py:3:1: I201 Missing newline before sections or imports.
bugzilla2gitlab/utils.py:4:1: I100 Import statements are in the wrong order. from xml.etree should be before import dateutil.parser
bugzilla2gitlab/utils.py:4:1: I201 Missing newline before sections or imports.

Now I'll rollback the flake8 cleanup commit because anyway these messages do not pertain to my changes. You could cherry pick and re-apply it if needed.

xmunoz commented 6 years ago

Ah yeah, I guess that setting hasn't been doing anything. Nice catch! I'll open a separate PR to add those checks to travis.