Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make a ppx_odoc that does the comment checking that ppx_js_style does #147

Open
lpw25 opened this issue May 31, 2018 · 12 comments
Open

Make a ppx_odoc that does the comment checking that ppx_js_style does #147

lpw25 opened this issue May 31, 2018 · 12 comments
Assignees

Comments

@lpw25
Copy link
Contributor

lpw25 commented May 31, 2018

ppx_js_style is the Jane Street style checking ppx. One of the things it checks is that documentation comments have valid syntax using the documentation comment parser from the old version of odoc (octavius).

Now that the parser is no longer a separate library ppx_js_style can't use an up-to-date documentation parser. Now that odoc is getting more widely used there are probably people who want to check their documentation comments without using the other parts of ppx_js_style. So my suggestion is to implement a stand-alone ppx that just checks the syntax of documentation comments.

At the moment I think it would have to live inside the odoc repository, since that is where the parser is available, but it could also be implemented separately if odoc added a parser library. Either approach seems fine to me.

@aantron
Copy link
Contributor

aantron commented May 31, 2018

Right now, it should be possible to make the parser sublibrary of odoc, in this repo, installable by just adding a (public_name odoc.parser) stanza to its jbuild file. ppx_odoc could then live in a separate repo, but depend on odoc.parser. I think model will also have to be installed, perhaps as odoc.model. For reference, right now, package odoc installs only the odoc binary.

@pmetzger
Copy link
Member

This seems like a very good idea.

@rizo
Copy link
Collaborator

rizo commented Oct 23, 2018

@aantron Dune currently does not permit having private dependencies for public libraries. This means that adding (public_name odoc.parser) would require making other odoc libraries public too.

@rizo rizo self-assigned this Oct 23, 2018
@rizo
Copy link
Collaborator

rizo commented Oct 24, 2018

@dbuenzli mentioned in #226 (comment) that parsing docstrings directly from mli files – which is what syntax rewriters does, of course – might not be reliable because other syntax rewriters might change the AST.

Is there a way to enforce the order of application of rewriters?

Is it possible to load a cmti file from a syntax rewriter for a given source file and only add warning attributes to the AST for merlin?

@lpw25
Copy link
Contributor Author

lpw25 commented Dec 3, 2018

Is there a way to enforce the order of application of rewriters?

You can give a position parameter to Migrate_parsetree.Driver.register to control the order the ppx passes are applied. However, using a non-zero position means that the AST must be processed an additional time which can slow down preprocessing. Personally, I think it would be fine to not worry about the order for this use case. It will still give warnings for any problems with user written comments, which is the main thing you want.

@avsm
Copy link
Member

avsm commented Dec 14, 2018

Just cross-referencing the Dune issue on having a public library depend on private modules: ocaml/dune#1017 -- this restriction can be lifted in the future if it's a blocker for this ppx odoc feature.

@aantron
Copy link
Contributor

aantron commented Dec 14, 2018

As a further note, we ended up exposing the sublibraries as described in #236 (comment). Of course, for a supported tool, we would want to fix the names, if the tool is developed separately.

@github-actions github-actions bot added the stale label May 1, 2020
@lpw25 lpw25 removed the stale label May 1, 2020
@github-actions github-actions bot added the stale label Jun 1, 2020
@lpw25
Copy link
Contributor Author

lpw25 commented Jun 1, 2020

I'm not sure where this bot has come from, but it's clearly set to use a too short time span. The first time it ran I carefully unmarked the issues that are still important to fix, but I can't waste time doing that every month. I can't see how this won't leave the repository with 0 open issues, which makes the issue tracker just as useless as when there are 200 open stale issues.

The aim should be to distinguish issues that people still care about and are likely to get solved from issues that people don't care about or will not likely get solved. Marking nothing as stale and marking everything as stale are equivalently bad approaches to addressing this.

@dbuenzli dbuenzli added not stale and removed stale labels Jun 1, 2020
@dbuenzli
Copy link
Contributor

dbuenzli commented Jun 1, 2020

@lpw25 I noticed there's a not stale label. I suppose you have to apply that.

But yes except on open ended/transient issue tracker like the opam-respository the way these new bots behave are user hostile and a poor substitute for a carefully curated issue tracker. Your typical programmer solution to a human problem, instead people should just learn to say "no"...

@Et7f3
Copy link

Et7f3 commented Jun 1, 2020

@lpw25 the new bot is a "user" associated with GitHub Action: https://github.com/ocaml/odoc/blob/master/.github/workflows/stale.yml. GitHub Action allow to do automatically bunch of task that maintainer have to do manually. It is a super-set from CI.

@lpw25
Copy link
Contributor Author

lpw25 commented Jun 1, 2020

@dbuenzli My opinion is less strong than that: I don't mind these bots generally -- I find the one on ocaml/ocaml useful for example -- but I think that this one is misconfigured. They generate an amount of work for people that is related to the time span for which things are considered stale. I don't mind that amount of work being non-zero -- having to do something once a year for each issue I care about is fine for me -- but once a month is far too high.

@ocaml ocaml deleted a comment from github-actions bot Jun 9, 2022
@ocaml ocaml deleted a comment from github-actions bot Jun 9, 2022
@dbuenzli
Copy link
Contributor

dbuenzli commented Jun 9, 2022

Is there still interest in this issue ? Isn't the solution to the problem to simply generate your docs ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants