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

Scripts optparse to argparse #2864

Merged
merged 60 commits into from Dec 15, 2016

Conversation

Projects
None yet
3 participants
@stevepiercy
Copy link
Member

commented Dec 11, 2016

refs: #2842

app = self.get_app(app_spec, self.options.app_name,
options=parse_vars(self.args[2:]))
app = self.get_app(app_spec, self.args.app_name,
options=self.args.config_vars)

This comment has been minimized.

Copy link
@mmerickel

mmerickel Dec 11, 2016

Member

missing parse_vars

parser = optparse.OptionParser(
usage,
parser = argparse.ArgumentParser(
prog="pviews",

This comment has been minimized.

Copy link
@mmerickel

mmerickel Dec 11, 2016

Member

drop prog argument

@@ -38,7 +38,8 @@ def _makeConfig(self, *arg, **kw):
def test_good_args(self):
cmd = self._getTargetClass()([])
cmd.bootstrap = (dummy.DummyBootstrap(),)
cmd.args = ('/foo/bar/myapp.ini#myapp', 'a=1')
cmd.args.config_uri = ('/foo/bar/myapp.ini#myapp',)

This comment has been minimized.

Copy link
@mmerickel

mmerickel Dec 11, 2016

Member

not a tuple

inst = self._makeOne()
inst.args = ['foo', 'a=1', 'b=2']
result = inst.get_options()
inst.args.config_args = ['a=1', 'b=2']

This comment has been minimized.

Copy link
@mmerickel

mmerickel Dec 11, 2016

Member

config_vars

@stevepiercy stevepiercy added this to the 1.8 milestone Dec 11, 2016

@stevepiercy stevepiercy self-assigned this Dec 11, 2016

@stevepiercy

This comment has been minimized.

Copy link
Member Author

commented Dec 11, 2016

ptweens and pviews are ready for review.

I'm stuck on pshell, and I don't know where to go next. I think I got pshell to run as follows. Using my local project from the starter scaffold, I did pip install -e . to pull in all the latest changes of the p* scripts, then tried to run ../../env35/bin/pshell development.ini. This started an interactive Python shell.

There are probably mistakes in my pshell, too, the latest of which is committed and pushed here.

In the tests (latest also committed and pushed), there's patch_* stuff in _makeOne(). I tried debugging the test, but I could not figure out where to look or what it does, so I don't know how to fix the tests.

@mmerickel

This comment has been minimized.

Copy link
Member

commented Dec 11, 2016

You need to fix the _makeOne method to do the right thing. It doesn't match up with all of the changes you're making.

@stevepiercy

This comment has been minimized.

Copy link
Member Author

commented Dec 12, 2016

All things pass, except docs continue to fail to build (due to Sphinx/docutils bug), but build locally. Hopefully Sphinx will make a new release with the fix.
https://travis-ci.org/Pylons/pyramid/jobs/183174559#L700

Ready for final review. Can you believe it?

@stevepiercy stevepiercy force-pushed the stevepiercy:scripts-optparse-to-argparse branch from 22db456 to ed1764d Dec 14, 2016

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented Dec 14, 2016

I'll review this in a little bit.

@bertjwregeer
Copy link
Member

left a comment

Just a simple comment, besides that, LGTM. @mmerickel I'll leave the merging to you in case you have any last minute requests.

@@ -51,11 +51,11 @@ def nothing(*arg):
extensions = [
'sphinx.ext.autodoc',
'sphinx.ext.doctest',
'repoze.sphinx.autointerface',

This comment has been minimized.

Copy link
@bertjwregeer

bertjwregeer Dec 15, 2016

Member

Is there a particular reasoning for the ordering of these? Or can they be alphabetized?

@mmerickel mmerickel merged commit 5b5b20b into Pylons:master Dec 15, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mmerickel

This comment has been minimized.

Copy link
Member

commented Dec 15, 2016

Thanks for powering through @stevepiercy

mmerickel added a commit that referenced this pull request Dec 15, 2016

@stevepiercy

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2016

Thank you for your guidance and patience. Hope the multiple facepalm wounds heal.

@stevepiercy

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2016

Closes #2804

@stevepiercy stevepiercy deleted the stevepiercy:scripts-optparse-to-argparse branch Dec 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.