[hibernate-dev] OGM: CheckStyle and imports due to JavaDoc comments

[Hardy Ferentschik]

On 1 Jan 2013, at 10:58 AM, Sanne Grinovero <sanne at hibernate.org> wrote:

> Let's relax this, the likelyhood of this introducing a real problem
> affecting the runtime is extremely low, and there are better ways to
> prevent that. I've reopened the issue.

Why? The few occasions where it happens a fully qualified class name does not hurt. 
It is not enough that it is considered bad practise and even Checkstyle for that 
reason does not allow it in the UnusedImports check?

Now we can argue forward and backward about what the compiler does, or how the
byte code looks like or what a JVM does or does not do, fact there is a potential risk.
Why not stay on the safe side.

> - we're arguing only about things used in javadoc but not code, this:
>   -- makes the probability to trigger very small

and it makes the occasions where I have to specify a fully qualified name in the 
javadocs also rare. In most cases the simple name be enough.

>   -- doesn't save us from screw ups anyway

?
> - IDEs don't help -> very annoying to find it out at pull request time

It does. Idea actually warns me about this case. 

> - it is only a problem if you're loading an optional dependency which
> shouln't be required for that specific component
>   -- again restricts the application very much

?

>   -- how likely is it that you're referencing a class which is so
> much out of context to not be even a dependency of the current class?

In Validator we have optional library integration with Jodatime, JSoup and 
Paranamer. Validator behaves differently or offers additional functionality 
in case any of the libraries is on the classpath. So I find the use case quite real.

> Now to restrict these chances to zero - as I know some of us are more
> perfectionists - I actually believe we should simply eradicate any
> concept optional dependency.


> The whole concept, as defined by Maven,
> is unreliable and tricky to handle.

+1 I agree, optional dependencies are badly handled by Maven. One of
its drawbacks. Does it make the use case of an "optional" dependency less
useful? IMO no

> We could explore lots of better
> ways, for example:
> # different Maven modules keeping the code dependency isolated to a
> specific module

I don't think that scales, see the HV case.

> # different source directories in the same M.Module to keep the
> source unit isolated + check with checkstyle what is legal to be
> imported with different rules on each path

-1 That's for sure not the Maven way

> I personally like the Maven module split better. I'd like to stress
> that we usually have a 1:1 match between the Maven artifacts that we
> produce during a release from modular source projects. This doesn't
> have to be the case, I think we could output jar files which make
> sense to a consumer which don't necessarily match 1:1 the source
> structure.

-1

> What I'm getting to is that code blocks which use Apache Avro should
> have their own isolated module, as the code which uses JGroups, or
> those depending to JPA (optional, this one gets tricky..); but we
> could (on a case by case) still wrap it all up in a single unit for
> the consumer if we don't want to expose a fragmented project.

Ok, so you are talking about creating a ueberjar using for example the 
shade plugin. Mind you that from a productisation point of view we are not
supposed to use this plugin (even though the real problem is relocating classes
which would not be the case here). 

Regarding modularisation, I actually support the idea of a separate serialisation 
module and maybe a module for remote backends (which would depend on the
serialisation module). However, I think it should be done as proper modules, meaning 
if you want to have an ORM based app where you want Search you will need
the Search engine, orm, serialisation and remote-backends (the last two modules are made 
up atm) modules. 

> For the record however I'm pretty sure that some older JVMs would have
> a problem with such a missing class definition, but I didn't say it
> would actually load the class: that's a different phase of linkage, as
> it needs to prevent circularity references.

So why not be safe than sorry?

--Hardy