cli: Introduce new `--format` argument (and remove `--xunit-file`) #14

dbaty · 2024-03-28T14:15:32Z

Any output (text or xUnit) is now directed to the standard output. It
will be easier to add new formats (such as CSV or JSON, which I plan).

TiphaineLAURENT · 2024-04-05T16:10:14Z

src/check_oldies/check_branches.py

+    ok_msg = err_msg = ""
    if has_old_branches:
        err_msg = "NOK: Some branches are too old."
-        print(err_msg)
    else:
-        err_msg = ""
-        print("OK: All branches are fresh.")
-    if out:
-        print(out)
+        ok_msg = "OK: All branches are fresh."


Plutôt que d'initialiser des variables qu'on va modifier ou non, on pourrait avoir une petite fonction get_messages(has_old_branches: bool) -> tuple[str, str] ?

Mouais, je ne suis pas convaincu. :) Là, on a bout de code de 5 lignes, simple, clair, direct. Je trouve ça plus parlant qu'un appel à une fonction.

TiphaineLAURENT · 2024-04-05T16:11:00Z

src/check_oldies/check_fixmes.py


    # Look for orphan FUTURE tags
-    orphan_futures = annotations.get_orphan_futures(config)
-    for orphan in orphan_futures:
-        out.append(warn(orphan_str(orphan)))
-        uncolorized_out.append(orphan_str(orphan))
+    orphan_futures = check_oldies.annotations.get_orphan_futures(config)

-    out = os.linesep.join(out)
+    ok_msg = err_msg = ""


Pareil que pour check-branches

TiphaineLAURENT · 2024-04-05T16:12:29Z

src/check_oldies/commands.py

+# we'll have to make sure that we can do that and still support 3.8
+# and 3.9). or perhaps get rid of it?


3.8 and 3.9 should not be supported as they are near the end of their security cycle and close to end-of-life : https://devguide.python.org/versions/

What are potential limitations regarding 3.8 and 3.9 ?

I am not inclined to remove support of versions that have not yet reached their end of life (however old they might be).

TiphaineLAURENT · 2024-04-05T16:15:34Z

src/check_oldies/output.py

+    if error_message:
+        lines.append(warn(error_message))


Pas de couleur spécifique aux erreurs pour les différencier des alertes ?

Qu'est-ce qu'une erreur, qu'est-ce qu'une alerte ?

On a 2 sortes de lignes :

lignes informatives : "t'as un FIXME, il a 3 jours, je t'informe, tout va bien"

lignes d'erreur : "t'as un FIXME, il a 200 jours, c'est une erreur"

Les premières sont affichées dans la couleur par défaut du terminal. Les secondes sont affichées en rouge.

J'ai peut-être mal compris mais je vois ok_message, error_message et must_warn alors j'ai pensé à 3 echelons

must_warn indique une ligne d'erreur : une annotation ou une branche trop vieille (ou un tag "future" orphelin).

src/check_oldies/output.py

TiphaineLAURENT · 2024-04-05T16:18:40Z

src/check_oldies/output.py

+    raise ValueError(f"Unknown output format: '{format_}'")
+
+
+def printer(objects: list, output_format, **options):


objects: list, output_format: OutputFormat

bien vu, c'est fait

TiphaineLAURENT · 2024-04-05T16:19:09Z

src/check_oldies/output.py

+    return xml.etree.ElementTree.tostring(suite, encoding="utf-8").decode("utf-8")
+
+
+def get_formatter(format_) -> typing.Callable:


output_format plutôt que format_ ?

sandre35 · 2024-04-12T16:15:15Z

tests/test_check_branches.py

@@ -113,17 +114,17 @@ def test_output_old_branches(capfd):  # capfd is a pytest fixture
    assert stdout == expected


-def test_xunit_file_generation(tmp_path):  # tmp_path is a pytest fixture
-    xunit_path = tmp_path / "xunit.xml"
+def test_xunit_file_generation(capfd):  # capfd is a pytest fixture


use linting instead of comment :)

sandre35 · 2024-04-12T16:17:13Z

tests/test_output.py

+# `xml.etree.ElementTree.tostring()` preserves attribute order only as of
+# Python 3.8. With prior versions, we cannot easily compare XML
+# output, so don't bother.
+HAS_UNSORTED_XML_WRITE = sys.version_info[:2] < (3, 8)


Python minimal version supported is 3.8. So we can remove all logic link to that

Oui, dans une autre pull request où je supprimerai le support de Python 3.7 (entre autres modernisations).

Any output (text or xUnit) is now directed to the standard output. It will be easier to add new formats (such as CSV or JSON, which I plan). If you used the `--xunit-file`, you must now use `--format` and redirect the standard output to a file.

`check-branches`, `check-fixmes` and `check-futures` commands now accept a `--format csv` argument. Example:

By default, all commands show all annotations, branches, etc., including those that are recent. When exporting to a CSV format, it's useful to filter out the recent items and only display old ones.

It's a (relative) path, not a mere filename.

dbaty · 2024-04-12T17:24:01Z

@sandre35 et @TiphaineLAURENT : J'ai pris en compte tous les commentaires (parmi ceux que je souhaitais prendre en compte ;) ), rebasé par rapport à master... et ai push-forcé comme un sauvageon. :) J'ai aussi corrigé divers oublis, comme la mise à jour du changelog et de la doc. La pull request m'a l'air à peu près complète, désormais, prête à être mergée.

dbaty force-pushed the dbaty/output_formats branch 8 times, most recently from 2525abb to 6517863 Compare March 28, 2024 16:54

dbaty marked this pull request as ready for review March 28, 2024 16:55

dbaty force-pushed the dbaty/output_formats branch 2 times, most recently from 3fc73ee to 9736311 Compare March 29, 2024 10:24

TiphaineLAURENT reviewed Apr 5, 2024

View reviewed changes

dbaty mentioned this pull request Apr 12, 2024

feat: Separate FUTURE flags detection in check-future-flags command #15

Merged

dbaty added 2 commits April 12, 2024 17:42

ci: Bump GitHub actions

e914bd3

docs: Fix markup

e43e3f3

sandre35 reviewed Apr 12, 2024

View reviewed changes

dbaty force-pushed the dbaty/output_formats branch 2 times, most recently from 2cf5881 to 29dbfb3 Compare April 12, 2024 17:02

dbaty force-pushed the dbaty/output_formats branch from 29dbfb3 to e39fa46 Compare April 12, 2024 17:18

dbaty added 3 commits April 12, 2024 19:21

cli: Add CSV format

a34cdf1

`check-branches`, `check-fixmes` and `check-futures` commands now accept a `--format csv` argument. Example:

cli: Add --only-old argument to all commands

22a478b

By default, all commands show all annotations, branches, etc., including those that are recent. When exporting to a CSV format, it's useful to filter out the recent items and only display old ones.

csv: Rename filename column as path in CSV output of check-fixmes

2f3cdb5

It's a (relative) path, not a mere filename.

dbaty force-pushed the dbaty/output_formats branch from e39fa46 to 2f3cdb5 Compare April 12, 2024 17:21

docs: Update configuration of ReadTheDocs

d479f31

dbaty force-pushed the dbaty/output_formats branch from 6cb8bae to d479f31 Compare April 12, 2024 17:28

sandre35 approved these changes Apr 14, 2024

View reviewed changes

dbaty merged commit d479f31 into master May 14, 2024
6 checks passed

dbaty deleted the dbaty/output_formats branch May 14, 2024 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: Introduce new `--format` argument (and remove `--xunit-file`) #14

cli: Introduce new `--format` argument (and remove `--xunit-file`) #14

dbaty commented Mar 28, 2024

TiphaineLAURENT Apr 5, 2024

dbaty Apr 12, 2024

TiphaineLAURENT Apr 5, 2024

TiphaineLAURENT Apr 5, 2024

sandre35 Apr 12, 2024

dbaty Apr 12, 2024

TiphaineLAURENT Apr 5, 2024

dbaty Apr 12, 2024

TiphaineLAURENT Apr 12, 2024

dbaty Apr 12, 2024

TiphaineLAURENT Apr 5, 2024

dbaty Apr 12, 2024

TiphaineLAURENT Apr 5, 2024

dbaty Apr 12, 2024

sandre35 Apr 12, 2024

dbaty Apr 12, 2024

sandre35 Apr 12, 2024

dbaty Apr 12, 2024

dbaty commented Apr 12, 2024

		# we'll have to make sure that we can do that and still support 3.8
		# and 3.9). or perhaps get rid of it?

		raise ValueError(f"Unknown output format: '{format_}'")


		def printer(objects: list, output_format, **options):

		return xml.etree.ElementTree.tostring(suite, encoding="utf-8").decode("utf-8")


		def get_formatter(format_) -> typing.Callable:

cli: Introduce new --format argument (and remove --xunit-file) #14

cli: Introduce new --format argument (and remove --xunit-file) #14

Conversation

dbaty commented Mar 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbaty commented Apr 12, 2024

cli: Introduce new `--format` argument (and remove `--xunit-file`) #14

cli: Introduce new `--format` argument (and remove `--xunit-file`) #14