[MSHARED-1326] Improve (documentation on) MavenReport interface

[michael-o]

A possible improvement w/o renaming any methods.

[michael-o]

@kriegaex

[michael-o]

@kriegaex I'd first to normalize the following:

shared report output directory

vs.

base report output directory

vs.

report base output directory

I believe that the last one is logically wrong. I hink it solely refers to the report itself and not the rest. It provides false information.
Now we have only two left. What is important to me is that plugin authors understand that this is not an exclusive directory. Maybe we need to combine them:

shared base report output directory

WDYT?

[kriegaex]

I believe that the last one is logically wrong.

I do not think so, because it dependes on the context and the assumptions under which users read the documentation. My context is obviously different from yours, which is why you think it is logically wrong. Maybe it is also a grammatical question to some degree. I prefer "report base (...) directory" to "base report (...) directory", as the latter seems to be bad English. It is a base directory. What kind of base directory? A report base directory, not a base directory for anything else. Which is why I would order the terms that way.

As for "a lot (of medicine) helps a lot" or combining the terms, making the result an even more unweildy "shared base report output directory" or "shared report base output directory" as the lesser evil, I am not sure if that might not be too subtle for most plugin developers to grasp. In such cases, a little bit of explanation usually helps, why I usually just expand the description, maybe even adding an instructive example. Precise and to the point is good, but sometimes too terse.

I leave the decision up to you, but you asked "WDYT?", so these are my two cents.

[michael-o]

@elharo As you are a native English speaker and we don't, would you mind to add your opinion on this?

[elharo]

This one is a little hard to describe because what the API does is complicated. On first read, I thought this was just a file name but it's not. But now I read it a third time and maybe it is just a file name? I don't know.

If it is just a file name, then "Returns the name of the output file that will be written in {@link #getReportOutputDirectory()}"

If it can contain subdirectories, then "Returns a path relative to {@link #getReportOutputDirectory()} where the output file will be written."

[kriegaex]

@elharo, what it contains (file or subdirectory) depends on whether it is a single or multi page report. Javadoc would be an example for multi page.

[elharo]

OK, so maybe it's a directory name? All of this should be laid out in the docs. What might help is a longish class comment explaining where the report files go and how they're configured.

[michael-o]

@elharo, it is both. A path to a file. I need to rename the API to satisfy this. I could do with a default interface method. The current name is just bad. I am happy to accept any reformulation of the docs which make it better. Should I raise a PR to rename the method and deprecate it? FWIW, it is not necessarily a file name because it is free of a format. The plugin impl does not decide about the format. It is a symbolic name in a virtual FS, so to speak.

[elharo]

I'm not sure the word "base" adds anything here

output of --> the output of

[kriegaex]

@elharo: It adds the notion that not necesarily all reports end up directly in that directory, but that they can also end up in a subdirectory. A base directory is the flipside of the medal regarding the term subdirectory. Of course, without the "base" prefix, that would still hold true, but like I said in an earlier comment: Maximally terse descriptions are not always ideal from a reader's perspective. BTW, the reader inevitably knows way less about the meaning of parameters than the provider. She is also unlikely to read all documentation, but only as little as possible to solve her problem or implement her use case. I.e., she does not have the full context. And even if she does, she is unlikely to remember all of it.

[michael-o]

@elharo I tried to apply all of your comments. Maybe the method has to renamed to:

String getOutputPath()

since Name is misleading...?

[michael-o]

Now added #getOutputPath() and deprecate #getOutputName().

[hboutemy]

good clarification

[michael-o]

good clarification

Is the method name change acceptable for you and reasonable?

[elharo]

LGTM

[michael-o]

@kriegaex, any further objections before I merge this one?

[kriegaex]

Thanks for the improvement. Like boyscouts, you guys in your joint effort left the camp ground behind cleaner than you found it. 🙂