I have modified the commits to take out the test suite and opened it as a new
PR https://github.com/rpm-software-management/rpm/pull/1242.
Am therefore closing this PR in favour of the new one.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or
Closed #1195.
--
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/1195#event-3374229415___
Rpm-maint mailing list
Don't bother. In the near future you can just use whatever you want for the
tests:
https://github.com/rpm-software-management/rpm/issues/1199#issuecomment-633979688
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> 1. [...] That's not really different from when people work on their own
> packages first and push it back upstream.
Hmm, I thought this was in a separate repo actually, but I see it's just Fedora
dist-git. Like noted in earlier comments, stuff developing their own
test-suites is a certain
Ok, sounds good. I'll separate out the test suite.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
At any rate, I'm totally fine with merging just the code right now and worry
about the rest later.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Maybe add a comment to the top of the script explaining the model: this is
developed and tested at repository at `` and rpm only maintains a
read-only copy of that, synced from time to time. At least that's how I
perceive this thing.
--
You are receiving this because you are subscribed to
As a short term thing, we just need to merge in the code, so other changes
don't diverge too much.
Not having the tests here is something that I'd rather avoid -- how do we
expect everybody to remember that the tests are somewhere else?
--
You are receiving this because you are subscribed to
Sorry for the holdup folks, I've been mulling over this quite a bit. The tests
that is.
The test-suite is a deal-breaker really. Rpm's test-suite is the autotest-based
thing, and everything in rpm needs to use that for tests. We can't have
individual bits and pieces bring in their own infra,
I'd like to see this merged.
--
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/1195#issuecomment-631572506___
Rpm-maint mailing
I would like to work on some more changes, for example to handle "extras", but
I'm not sure it's a good idea before this is merged.
CC @Conan-Kudo, @pmatilai, @ffesti
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Since the high level discussion of whether this shall be split or not, can we
please merge this for now to avoid the base diverging?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
These proposed changes have been merged to Fedora rawhide:
https://src.fedoraproject.org/rpms/python-rpm-generators/pull-request/15
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
This pull request **introduces 1 alert** when merging
90e68fa8f27a594067763476e69c723ab8beb971 into
022b48d21092f8a79103fa9318376fb26911e571 - [view on
LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-6f63a05ddcb5d681a51a46456602e60a7916851b)
**new alerts:**
* 1 for
I've merged the first fixup. Leaving the second one pending depending on
whether we decide to merge pythondistdeps here or split off to a new package.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
While the [discussion about possibly moving pythondistdeps to a different
repo](https://github.com/rpm-software-management/rpm/issues/1199) goes on:
Any technical issues or concerns with the PR?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly
@torsava commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the commit
@hroncok commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the commit
@Conan-Kudo commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the
@torsava commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the commit
This pull request **introduces 1 alert** when merging
e7cc87a083366439c0450c720ed36475b09d into
9a30bcdacca365a6d8c4078dc6b6f8014d31d66f - [view on
LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-6a72d51c59c7dece06617335d467976326e344fd)
**new alerts:**
* 1 for
> Any new sources need to be EXTRA_DIST in Makefile.am's.
I've added the checked-in files to the `EXTRA_DIST` in the fixup commit. I
didn't add the automatically-downloaded metadata, as the test script will
automatically fetch them if missing. If it's desirable to have them inside
EXTRA_DIST
@torsava pushed 1 commit.
e7cc87a083366439c0450c720ed36475b09d fixup! scripts/pythondistdeps: Add
the tests to the readme and makefile
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
This pull request **introduces 1 alert** when merging
96be70abdb8f5b7633fb41829a8b5d496a466e03 into
9a30bcdacca365a6d8c4078dc6b6f8014d31d66f - [view on
LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-37240692b220d858f7c84048312ded371ecb5b9f)
**new alerts:**
* 1 for
@torsava commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the commit
@torsava pushed 1 commit.
96be70abdb8f5b7633fb41829a8b5d496a466e03 fixup! scripts/pythondistdeps: Add
option to generate major-version provides only for specified Python versions
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
@Conan-Kudo commented on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group =
@torsava commented on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group =
> Any new sources need to be EXTRA_DIST in Makefile.am's. For a moment I was
> puzzling as to how come this is passing dist-check when the added sources are
> missing in the release tarball, but then realized the added tests are not
> getting run in _our_ test-suite. Which to me kinda steps
@Conan-Kudo commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the
@hroncok commented on this pull request.
> @@ -11,6 +11,10 @@
# RPM python dependency generator, using .egg-info/.egg-link/.dist-info data
#
+# Please know:
+# - Notes from an attempted rewrite from pkg_resources to importlib.metadata in
+# 2020 can be found in the message of the commit
@Conan-Kudo commented on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group =
@hroncok commented on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group =
@Conan-Kudo commented on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group =
@torsava commented on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+group =
@pmatilai I wish you were right, but in practice that is not what has happened.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> Ultimately, the problem is that it not being in the rpm tarball means I have
> no hammer to get people to discover it and use it. Unfortunately, the only
> standardization point is in rpm itself.
I would argue the opposite: people interested in Python are more likely to find
and contribute
@hroncok Ultimately, the problem is that it not being in the rpm tarball means
I have no hammer to get people to discover it and use it. Unfortunately, the
only standardization point is in rpm itself. I hesitate to split it out after
spending so much work integrating it into rpm upstream to get
Having a separate repo here is actually not a compromise but IMHO the preferred
option. rpm-extras was thought more as an interim solution for scripts that
don't have a large enough contributor base to be projects on their own.
Even better if people from multiple distributions can share the
Totally fine by me. @torsava @Conan-Kudo @gordonmessmer @ignatenkobrain
@s-t-e-v-e-n-k WDYT?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> Maybe there might be a compromise by having a separate repo in the
> rpm-software-management namespace?
That could be an option, and would be absolutely fine by me if it makes sense
for the Python "SIG" around here. rpm-extras was supposed to be the place for
scripts like these, but it's not
> We talk about upstreaming changes like this, but to me the situation looks
> pretty much the opposite: this has an active upstream of its own and we're
> merely syncing it from time to time, which is doing double work for ... I
> don't know what.
We are happy to maintain this ourselves. The
Any new sources need to be EXTRA_DIST in Makefile.am's. For a moment I was
puzzling as to how come this is passing dist-check when the added sources are
missing in the release tarball, but then realized the added tests are not
getting run in *our* test-suite. Which to me kinda steps over a
@Conan-Kudo requested changes on this pull request.
> +
+
+if __name__ == "__main__":
+"""To allow this script to be importable (and its classes/functions
+ reused), actions are performed only when run as a main script."""
+
+parser = argparse.ArgumentParser(prog=argv[0])
+
@hroncok approved this pull request.
FWIW I've reviewed this before the PR was opened.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
> **new alerts:**
>
> * 1 for Module is imported more than once
It's a lazy import so that if it's not needed, it doesn't slow down the
execution.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
This pull request **introduces 1 alert** when merging
dbc365b59207f4b00624241ae25542dd3f0a83cd into
ead227f2da98c223e4aa4a864f422722c1eb1e3c - [view on
LGTM.com](https://lgtm.com/projects/g/rpm-software-management/rpm/rev/pr-2b24124a3278ffa623c1ef2c5b3fee7bf9b950c5)
**new alerts:**
* 1 for
Please do a review commit-by-commit. I changed indentation of a large part of
the code, so the overall diff is hard to read.
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1195
-- Commit Summary --
*
48 matches
Mail list logo
