Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
Merged #1181 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#event-3243373851___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
This very much breaks my pull request I'm preparing — it seems that once you start working on pythondistdeps, everyone gets notification about it and starts working on it too :) — but it's beautiful and I'm all for it! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614615287___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
@Conan-Kudo approved this pull request. Someone finally did something that has been on my TODO for me. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#pullrequestreview-394558113___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
Right, so it's an [upstream Python guideline](https://www.python.org/dev/peps/pep-0008/#blank-lines) that I wasn't even aware of. I'll just agree to disagree with that then, so nevermind :smile: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614529001___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
Yes, flake8 warns on needing two blank lines, not one. https://www.flake8rules.com/rules/E302.html -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614517974___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
While on the subject - are all those added empty lines due to some flake8 warnings? I like visual aides as much as the next guy, but I do disagree with wasting perfectly good screen estate with two consecutive empty lines when one will do the trick already :grin: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614515594___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
Splitting to a separate commit would be great, no need to make a separate PR though. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614512778___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
Would you like me to revert that part of the change? They are fairly self-contained, so easy enough to pull out in a separate change. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614494499___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
Nice, but please don't mix whitespace changes with code changes. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181#issuecomment-614462764___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
@s-t-e-v-e-n-k pushed 1 commit. 0afbfbb67b8b5d8346271a7d8d2351ad4ed0f411 scripts/pythondistdeps: Switch to argparse -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1181/files/0ea13f93d5b179c416042274d80203c8ca6ae24f..0afbfbb67b8b5d8346271a7d8d2351ad4ed0f411 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
@ignatenkobrain commented on this pull request.
> +parser.add_argument('-P', '--provides', action='store_true', help='Print
> Provides')
+parser.add_argument('-R', '--requires', action='store_true', help='Print
Requires')
+parser.add_argument('-r', '--recommends', action='store_true', help='Print
Recommends')
+parser.add_argument('-C', '--conflicts', action='store_true', help='Print
Conflicts')
+parser.add_argument('-E', '--extras', action='store_true', help='Print Extras')
Sure, but since we are now doing refactor of this code, it makes sense to
implement things like this so it is not possible to pass bunch of parameters
which together do not make sense.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1181#discussion_r409303168___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
@s-t-e-v-e-n-k commented on this pull request.
> +parser.add_argument('-P', '--provides', action='store_true', help='Print
> Provides')
+parser.add_argument('-R', '--requires', action='store_true', help='Print
Requires')
+parser.add_argument('-r', '--recommends', action='store_true', help='Print
Recommends')
+parser.add_argument('-C', '--conflicts', action='store_true', help='Print
Conflicts')
+parser.add_argument('-E', '--extras', action='store_true', help='Print Extras')
Ahh, now getopt didn't do that! :-P But sure, let me look at doing that.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1181#discussion_r409302119___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] scripts/pythondistdeps: Switch to argparse (#1181)
@ignatenkobrain commented on this pull request.
> +parser.add_argument('-P', '--provides', action='store_true', help='Print
> Provides')
+parser.add_argument('-R', '--requires', action='store_true', help='Print
Requires')
+parser.add_argument('-r', '--recommends', action='store_true', help='Print
Recommends')
+parser.add_argument('-C', '--conflicts', action='store_true', help='Print
Conflicts')
+parser.add_argument('-E', '--extras', action='store_true', help='Print Extras')
I tihnk this should be moved to its own mutually exclusive group.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1181#pullrequestreview-394319810___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
