minor 5.2 patches for pretty-printing

ocaml / ocaml

The core OCaml system: compilers, runtime system, base libraries

https://ocaml.org

Other

5.19k stars 1.06k forks source link

Closed chetmurthy closed 2 weeks ago

chetmurthy commented 2 weeks ago

chetmurthy commented 2 weeks ago

These are such trivial changes, I don't think it needs a Changes entry, does it?

chetmurthy commented 2 weeks ago

The Appveyor build failed with some "mingw64" error. Don't think it's related to my changes.

MisterDA commented 2 weeks ago

The Appveyor build failed with some "mingw64" error. Don't think it's related to my changes.

It's not related to your changes. The (temporary) patches of #13069 have not been picked up in the 5.2 branch.

gasche commented 2 weeks ago

With apologies for being a bother:

could you target trunk instead? I prefer to merge only in trunk, and then cherry-pick later.
it is also okay to say "this does not deserve a Changes entry", and then we add a label that our CI recognizes and it becomes happy; but as you are fixing a (small) bug, having an entry is nice, if someone were to hit the bug in the future they would have more chances to see that it was fixed and when (this is assuming that your Changes entry is clear enough on what issue it fixes)

Octachron commented 2 weeks ago

Since raw identifiers were introduced in 5.2 , a good compromise could be to add the current PR number to the raw identifier entry in the changelog.

Octachron commented 2 weeks ago

@chetmurthy , could you add tests for your fixes in testsuite/tests/parsetree/source.ml ?

chetmurthy commented 2 weeks ago