Pablo Correa Gomez authored 10 months ago and Pablo Correa Gomez committed 10 months ago

This is unnecessary, because the code does not use the version. And
setuptools was getting very confused with "mrhlpr.version", as it is
importing the top-level mrhlpr.py file, and failing to import the
module.

Pablo Correa Gomez authored 10 months ago and Newbyte committed 10 months ago

Fixes #17

As it will fail, and that's annoying!

Fixes #18

This should reduce the risk of changes being missed in refactors like
the one discovered by Zach DeCook which was fixed in
7fce8005.

Some are still missing, but it's a start.

mypy cannot find these as they're defined in __init__.py:

$ mypy mrtest.py mrhlpr.py --check-untyped-defs
mrtest/zap_packages.py:36: error: Module has no attribute "is_root_user"  [attr-defined]
mrtest/zap_packages.py:37: error: Module has no attribute "get_sudo"  [attr-defined]
mrtest/add_packages.py:60: error: Module has no attribute "get_virtual_group"  [attr-defined]
mrtest/add_packages.py:63: error: Module has no attribute "is_root_user"  [attr-defined]
mrtest/add_packages.py:64: error: Module has no attribute "get_sudo"  [attr-defined]

This confuses mypy as bytes.decode() returns a str and
subprocess.run(...).stdout returns a bytes object.

While it would be possible to type output as being a Union[bytes, str],
these are really two distinct purposes and I think just renaming the
first use of output to output_raw helps make the code clearer anyway.

Even with the 5 second sleep, it sometimes seems to fail at setting auto
merge, and instead raises GitlabMRClosedError for some reason. As such,
retry up to 3 times with a 5 second sleep in-between, which seems to be
more robust from my testing.

While I understand the intent behind the original message, I always end
up reading it as that providing a thank you comment is discouraged.
While I think the intended meaning is obvious if you think about it, to
me writing it this way feels like a better way to communicate it.

In some cases you may not want to approve an MR but still merge it. For
example, if it's your own MR.

Even if automatic detection of self-merging later is implemented, there
may still be instances where you wish to merge an MR without approving
it, for example if an MR you don't understand well enough to feel
confident merging already has enough approvals for a merge.

Previously you'd just get an error that GITLAB_TOKEN wasn't set in the
environment without any information about how to resolve this.

py3-gitlab is now in community:
https://gitlab.alpinelinux.org/alpine/aports/-/commit/b470cf0780124804e17ce7510418b77640a42b0b

[ci:skip-build]: already built successfully in CI

Ruff will recursively lint all files in subdirectories if no list of
files is omitted. This also lets us remove the ShellCheck ignore as the
affected code no longer exist.

[ci:skip-build]: already built successfully in CI

If the list of files is omitted, Ruff will check all files recursively.

Use --diff to get a diff of what's not complicit instead of applying
the changes directly.

This does not cover the entirety of mrhlpr's codebase, and does not even
attempt to start typing mrtest. The types were generated with
MonkeyType, which seems to have worked well to get a starting point for
typing. However, it still required significant manual adjustment and as
such time investment due to some of the type hints being wrong
(presumably since MonkeyType runs off of runtime information and I
didn't manage to trigger every possible code path), but primarily due to
the codebase needing other fixes to make mypy happy.

Overall, I think MonkeyType saved me enough time to be worthwhile using
for future type expansion, but the time investment required is still
significant.

This also introduces the MergeRequestStatus and GitLabOrigin dataclasses
to avoid unnecessary type assertions and improve ease of reading the
code.

Nikita Travkin authored 1 year ago and Pablo Correa Gomez committed 1 year ago

mrtest installs packages from apk files which pins the package version
to the hash of the package. This means that apk would be reluctant to
update packages on which .mrtest- virtual package depends.

This can cause problems to the users who may forget that they have some
packages installed using mrtest, espacially if those were boot-critical
packages.

Add a script that would implement an apk hook that would nag the user
about installed mrtest packages. This hook should be installed in
/etc/apk/commit_hooks.d/ and will be run by apk on every execution.

Fixes: #13

I forgot to do this when migrating from Flake8 to Ruff.

Fixes b09df9bd

flake8 was dropped in b09df9bd, and
this is a left-over that allows us to drop the setup.cfg file

Only 3.8 is not EOL, and it's just some months. Even Debian old stable
is currenly on 3.9

According to
https://wheel.readthedocs.io/en/stable/user_guide.html#building-wheels
universal = 1 is opt-in

See similar patch in pmbootstrap for rationale:
https://gitlab.com/postmarketOS/pmbootstrap/-/commit/27618d5ffdb0b6cbf9471dc01a2ed14e1655ced0

We use this in pmbootstrap, and it is currently necessary to use Ruff.
Issue about moving Ruff to community:
https://gitlab.alpinelinux.org/alpine/aports/-/issues/15693

So we don't need to repeat it.

Repurposing argcomplete as a boolean telling whether the module
argcomplete was found results in a variable with two distinct purposes
and types, which is bad practice and confuses Mypy. As such, don't
assign False to argcomplete if the module is not found and instead just
check for whether it exists in sys.modules to determine whether it was
imported successfully.

This works in practice, but Mypy does not understand it, and I'm not
sure how to best resolve it.

`mrhlpr status 123` on a non-checked-out branch would panic because
`commits_follow_format` was an undefined variable.