Skip to content

Conversation

@jtulach
Copy link
Contributor

@jtulach jtulach commented Mar 1, 2016

As an initial response to #42 I am trying to get under control what is and isn't in the Truffle API. The rule is simple: "Every element in the API needs to have Javadoc with @SInCE tag". The rule builds on following observations:

  • Obviously having elements in the API without Javadoc is wrong
  • with Javadoc being there, the additional requirement to add one more line @since X.Y is simple
  • Developers are used to read Javadoc that contains @since tag - just like JDK does

We already have sigtest, but its biggest strength is in its binary compatibility check. It cannot verify presence of Javadoc and while it can be set to do exact API check, it has the drawback of keeping its info separate file(s) and not providing any of its information to developers reading Javadoc. As such I propose to use slight modification of our doclet to check that everything that is in the API is intended to be there.

This pull request is result of running the tool (via mx javadoc --unified) and doing additional manual changes (related mostly to default constructors). It assigns @since 0.8 or earlier to all elements. The consensus was that it doesn't make sense to go deeper into the history of the API than 0.8. After a quick poll the people around me have chosen the text 0.8 or earlier as the best among pre-0.8, <0.8, 0.1-0.8, (0.1,0.8] alternatives. Mostly because the part of the text before first space really represents a version.

I would prefer to merge the change as soon as possible, as the amount of changes is likely to contribute to higher probability of merge conflicts. In any case I plan to work on improvements - use 0.9, 0.10, 0.11 and 0.12 for elements introduced after 0.8 version.

Btw. The request to document every API element needlessly increases verbosity of the source code in once case: on default constructors. True, I had to add few of them manually. On the other hand I also discovered ~five errors where default constructors were left in the API and clearly that wasn't the intention. By requiring @since tag being present we'll warn every Truffle API developers of this kind of errors - if that can make the Truffle API better at least in one case (and I am sure of that), I don't mind how much verbose our source code gets.

Please, review my diff and help me find places where my automatic tool made mistakes.

* Location is never set to {@code null} and initialized before it is read.
*
* @since 0.8 or earlier
* @since 0.8 or earlier
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate?

@chumer
Copy link
Member

chumer commented Mar 1, 2016

Looks good except for one minor duplication (you can fix this in the next run)

I'd have preferred to do the final change (including 0.9, 0.10, 0.11, 0.12) in one PR to minimize the disruption of other PRs (one time disruption). But I trust you had technical reasons to do it this way. Should not block this PR.

@chumer chumer added the accept label Mar 1, 2016
@woess
Copy link
Member

woess commented Mar 1, 2016

I still think spreading @since on every member when its version is the same as the one in the @since of the class is a bad idea. It's not how it's done in the JDK, and I don't see a good enough reason for it.

@jtulach
Copy link
Contributor Author

jtulach commented Mar 1, 2016

I am trying to find out the duplications via grep, or other tool right now.

@jtulach
Copy link
Contributor Author

jtulach commented Mar 1, 2016

Output of

truffle$ grep -r @since -C 1 * | uniq -D

is now empty. Thus I believe the duplications are gone.

@jtulach
Copy link
Contributor Author

jtulach commented Mar 1, 2016

Yes, it is true that JDK isn't placing @since tag on every element of their API, but in spite of that I believe there are good reasons for adopting policy that every element in the API needs to have Javadoc with @SInCE tag. It helps us catch and discover errors in the API in a very natural way. We need such tool as there were many errors already and without tough discipline enforced by a tool, there would be even more in the future.

@jtulach
Copy link
Contributor Author

jtulach commented Mar 1, 2016

Accepted. I'll make sure all checks pass; squash the commits and merge.

@chumer chumer added accept and removed accept labels Mar 1, 2016
…as the oldest version. Assigning newer versions to well known APIs introduced later.
jtulach pushed a commit that referenced this pull request Mar 2, 2016
Making sure all API elements have @SInCE tag
@jtulach jtulach merged commit 1563406 into oracle:master Mar 2, 2016
@jtulach jtulach deleted the SinceTag branch March 2, 2016 02:13
dougxc pushed a commit that referenced this pull request Jun 15, 2016
…E.COM/truffle:feature/inst-test-lang to master

* commit '68779d26372a537482dff18231237162b002cc13':
  InstrumentationTestLanguage;  use LanguageError as the cause for IOException.
  InstrumentationTestLanguage: make a bit more friendly for tool development use by giving a "name" that is easier to type
  InstrumentationTestLanguage:  make a bit more friendly for tool development use by throwing IOException instead of RuntimeException for syntax errors.
zakkak added a commit to zakkak/mandrel that referenced this pull request Aug 6, 2020
zakkak added a commit to zakkak/mandrel that referenced this pull request Aug 7, 2020
zakkak added a commit to zakkak/mandrel that referenced this pull request Aug 10, 2020
zakkak added a commit to zakkak/mandrel that referenced this pull request Aug 12, 2020
zakkak added a commit to zakkak/mandrel that referenced this pull request Aug 26, 2020
galderz pushed a commit to galderz/mandrel that referenced this pull request Sep 2, 2020
jerboaa pushed a commit to jerboaa/graal that referenced this pull request Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants