wilkinsona / project-generator

3 stars 4 forks source link

Non deterministic order for plugin contribution #20

Open snicoll opened 5 years ago

snicoll commented 5 years ago

When contributing an additional plugin, the order by which it is added to pom.xml is determined by the order in which contributors are processed. It would be nicer if we had a way to add a plugin in a particular place.

This can be a corner case but:

  1. It eases testing when several contributors are at play
  2. Unfortunately, some plugins have to be ordered when they share the same phase and one has to run before another
wilkinsona commented 5 years ago

I think 1 can be addressed by testing at an appropriate level and testing robustly. If a plugin doesn’t care about its ordering, the test should not care either.

2 is a good reason to do this though. However, I’d prefer to wait until we have a need for it. I haven’t encountered one yet. Do you have one?

snicoll commented 5 years ago

I think 1 can be addressed by testing at an appropriate level and testing robustly. If a plugin doesn’t care about its ordering, the test should not care either.

Correct but the current based assertions don't. One test started to fail once I've moved to modules because a test expected the restdocs plugin to be third in the list. I agree we can improve the assertion to not need this at all.

However, I’d prefer to wait until we have a need for it. I haven’t encountered one yet. Do you have one?

Initializr doesn't have this use case. However, for styling purpose I'd like us to be ablte to order the plugins so that it matches the style we want to promote.

wilkinsona commented 5 years ago

Perhaps we could order the plugins by phase for now? It would centralise the solution and avoid individual contributions from having to worry about it. We can consider ordering within a phase when the need arises.

snicoll commented 5 years ago

I am not sure. I think our style is much about order by groupId / artifactId. I agree ordering within a phase is something we should add later.

So ordering lexicography (kind of) has my vote. It's the same with dependencies group and imports.

wilkinsona commented 5 years ago

Dependencies are sorted first by type (mapped to a scope in Maven and a configuration in Gradle) and then by coordinates. I think that’s the right way round to do it. Ordering plugins primarily by coordinates doesn’t feel right to me. It isn’t consistent with how we order dependencies and it isn’t how I’d write them in the pom. I’d rather order them primarily by phase so that they can be read in roughly the order of Maven’s lifecycle. Within a phase they can be ordered by coordinates for now until we have a requirement for something more. One complication is plugins where there’s no explicit phase configured. I’d put these either all at the start or all at the end.

snicoll commented 5 years ago

One complication is plugins where there’s no explicit phase configured.

Another complication is plugins for which there is more than one phase configured. I like the idea of ordering them by phase and then lexicography within a phase. However, goals have a default phase and it's kind of weird to have to know the phase for the sole purpose of ordering (overriding the phase a goal should run at looks a corner case to me).

My reasoning about ordering them lexicography is that you can easily spot common groups (i.e. all core plugin, all "Spring-related" plugins, etc. And what I mean to say with dependencies and imports is that we do the same thing as well (we group imports for certain prefix together and separate them with a new line, idem for dependencies).

Anyway, the purpose of this issue is to make sure we have a deterministic order. I am happy to take whatever order makes sense without having to define things we're not supposed to.

wilkinsona commented 5 years ago

Ok, let's sort by groupId and then artifactId.

snicoll commented 5 years ago

I have a feeling it would be a good idea to use a container like we've done for other parts of the build. That would allow us to add a plugin "before" another one for instance. It would also give a more consistent experience and we could improve the API around plugin without having to touch the main build object.