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

Critical: Build blocked by custom taglets #909

Closed
johnmay opened this issue Sep 13, 2022 · 8 comments
Closed

Critical: Build blocked by custom taglets #909

johnmay opened this issue Sep 13, 2022 · 8 comments
Labels

Comments

@johnmay
Copy link
Member

johnmay commented Sep 13, 2022

Just tried to do a release but got stuck with the custom taglets... (cdk-build-util) which I've been putting off updating. The APIs for all of these changed in Java 9+, so basically it all needs a rewrite. IIRC some of the functionality is now gone completely (or at least very poorly documented) and so it is a non trivial task to update. I certainly don't have time. Now previously I would just do a release with Java 8 and all was OK - however we now have a Java 11 module (iordf) due to JENA needing it.

My preferred solution would be to drop all the custom JavaDoc stuff, it's been a pain for a long time. However there are some nice to have features, like the cite, dictref stuff. The bugs tag is kind of redundant IMHO but sure I guess also "nice to have". What I think makes more sense is inline the data from these tags as Plain Old JavaDoc/HTML (perhaps leaving some tag in place if someone ever wants to put reverse it and them back in).

For example:

/**
 * Finds the Set of all Rings. This is an implementation of the algorithm
 * published in {@cdk.cite HAN96}. Some of the comments refer to pseudo code
 * fragments listed in this article. The concept is that a regular molecular

would become

/**
 * Finds the Set of all Rings. This is an implementation of the algorithm
 * published in <a data-cdk-cite="HAN96" 
 * href="https://pubs.acs.org/doi/abs/10.1021/ci960322f">
 * Hanser, Th. et. al.. J. Chem. Inf. Comput. Sci.. 1996. 36)</a>. Some of 
 * the comments refer to pseudo code fragments listed in this article. 
 * The concept is that a regular molecular
@johnmay
Copy link
Member Author

johnmay commented Sep 13, 2022

Looking back I think I have most of the patch for JDK11 taglets, but had issues with some of them. I'll look tomorrow with a fresh head but I still think simpler would be better here.

@johnmay
Copy link
Member Author

johnmay commented Sep 14, 2022

Build is ok, I would still like your view @egonw on inlining the data in the doc. At the moment I find it less convenient to add a citation for example.

We have a bespoke JavaDoc process which needs special treatment. Which is a bit of a pain, keeping things simple is more maintainable long term.

On a related note the new JDK 17 javadoc layout is very package focused and highlights the need for some restructuring - ala #626

@uli-f
Copy link
Member

uli-f commented Sep 15, 2022

I do think keeping things simple usually pays off in the long run. Whenever I used niche tools/functionality most caused painful issues at one point or where just not supported any longer (which then makes you drop them or migrate to sth else).

Inlining the citations using Plain Old JavaDoc/HTML probably makes them less fun to write, but I'd still consider this a preferred solution if it makes things simpler.

@egonw
Copy link
Member

egonw commented Sep 15, 2022

The introduction of the Taglets at the time actually simplified and improved our documentation. It solve a number of issue:

  • inconsistent references (and actually often no references, because too much work to add)
  • opens route to create a CDK knowledge base (JavaDoc to generate RDF)
  • harmonize documentation (people were using all sorts of styles, which made it hard to follow for users)
  • support development

There are two common pattern in our use of taglets:

  1. can we make the CDK JavaDoc more about cheminformatics, and not just about the Java parts.
  2. to support the CDK development

For example:

  • the @cdk.bug helped us with development. Right now, issue trackers and integration is so good, we can drop this. For example, remember that at the time we often had open bugs for classes that we needed to inform users about
  • the @cdk.cite bug remains relevant: consistent formatting of references, option of a bibliography, the knowledge graph (a long standing personal wish is to have website like EuropePMC state (see their PubLinks) "this article is implemented in CDK 2.8, see http://...cdk...javadoc/SomeClass.html"
  • the @cdk.dictRef has a similar goal: to link implementations to ontologies, see CHEMINF ontology and QSARML papers. Technologies have improved, and this can be done differently too. It just won't show up easily in the JavaDoc
  • the @cdk.IOOptions need a way to show up in the documentation. Ideas for alternatives welcome.
  • I think @cdk.inchi was for unit tests. to indicate which chemical structure it was about. and in this way get some idea of the chemical space covered by the algorithms in the CDK. I think this can be dropped :) We have better ways of doing this now.
  • @cdk.module has already been replaced by Maven modules. Think here the Get the bundled JAR size under control #911. This metadata was essential to modularize the development systems.
  • @cdk.thread -> well, clear what these are about. This is important for the user. Mind you, most of the CDK was not thread safe. I think it is now the default, and when it is not, it is a bug (@johnmay, correct?). So, this one was about important communication to the user but also about the CDK development, to highlight what parts still needed making thread safe.

So, generally, they made things easier, sometimes communication to users, sometimes the development of the CDK.

@johnmay
Copy link
Member Author

johnmay commented Sep 15, 2022

I think the module/githash one as actually still potentially useful as otherwise you can't know in the JavaDoc where stuff is. Aligning the packages with the modules would make this less of an issue.

I'm kind of happy I got things working but I still maintain it's an extra headache/inconvenience for building the project. I should try and get cdk-build-util on maven central though.

@johnmay johnmay closed this as completed Sep 15, 2022
@egonw
Copy link
Member

egonw commented Sep 15, 2022

get cdk-build-util on maven central

I can help with that.

@johnmay
Copy link
Member Author

johnmay commented Sep 15, 2022

I think should be easy since we already have the org.openscience.cdk domain working. Just needs a bit of cleanup, remove the EBI repo etc. It does still work but central is obv better.

@uli-f
Copy link
Member

uli-f commented Sep 15, 2022

I think should be easy since we already have the org.openscience.cdk domain working. Just needs a bit of cleanup, remove the EBI repo etc. It does still work but central is obv better.

I do remember having a bit of an issue downloading from the EBI repo when building the CDK some months ago.
So getting rid of that repo being part of the build requirements seems worthwhile to me.

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

No branches or pull requests

3 participants