search

repaint-io / maven-tiles

Injecting maven configurations by composition rather than inheritance
155 stars 32 forks source link

I feel the the statement 'closest first' is not true at all #111

Open stickycode opened 4 years ago

stickycode commented 4 years ago

Given a tile group:c with nested tiles

<project>
  <tiles>
    <tile>group:X</tile>
    <tile>group:Y</tile>
  </tiles>
</project>

and a tile group:t

<project>
  ...
</project>

and a project group:a with parent group:p

<project>
 <plugins>
   ... tile plugin
    <tiles>
      <tiles>group:c</tiles>
      <tiles>group:t</tiles>
    </tiles>
  </plugins>
</project>

The resolution order is p <- Y <- X <- t <- c

When I think the order should be p <- t <- Y <- X <- c

Note that if you reverse the order of c and t and a project group:a with parent group:p

<project>
 <plugins>
   ... tile plugin
    <tiles>
      <tiles>group:t</tiles>
      <tiles>group:c</tiles>
    </tiles>
  </plugins>
</project>

The resolution order is p <- Y <- X <- c <- t

Because of this its not possible to get the correct ordering of configuration when mixing direct tiles with indirect tiles

stickycode commented 4 years ago

The reason its not closed first is because X and Y are always ahead in the lifecycle than c and t, its not possible to run something from t before X or Y in the same phase which is incredibly annoying.

stickycode commented 4 years ago

Running tile plugin 2.16, java 11.0.7

rvowles commented 4 years ago

Could you suggest a PR to fix it? You may also wish to upgrade to 2.17 to get rid of the annoying java 11 warnings.

On Wed, Jul 1, 2020 at 10:44 PM Michael McCallum notifications@github.com wrote:

Running tile plugin 2.16, java 11.0.7

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/repaint-io/maven-tiles/issues/111#issuecomment-652343451, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAANTWG4QLR4LLCV53CKOATRZMHR3ANCNFSM4ONI7VLQ .

--

Richard Vowles, Full stack - from Kubernetes, through Node & Java, Web and Mobile development in Flutter - software developer for hire!

ph: +64275467747

stickycode commented 4 years ago

I can suggest a PR. Upgrading to 2.17 in progress.

On Thursday, July 2, 2020, Richard Vowles notifications@github.com wrote:

Could you suggest a PR to fix it? You may also wish to upgrade to 2.17 to get rid of the annoying java 11 warnings.

On Wed, Jul 1, 2020 at 10:44 PM Michael McCallum <notifications@github.com

wrote:

Running tile plugin 2.16, java 11.0.7

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/repaint-io/maven-tiles/issues/111# issuecomment-652343451, or unsubscribe https://github.com/notifications/unsubscribe-auth/ AAANTWG4QLR4LLCV53CKOATRZMHR3ANCNFSM4ONI7VLQ .

--

Richard Vowles, Full stack - from Kubernetes, through Node & Java, Web and Mobile development in Flutter - software developer for hire!

ph: +64275467747

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/repaint-io/maven-tiles/issues/111#issuecomment-652646072, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG4GFFSYUGPP6J55YDWRR3RZOQIDANCNFSM4ONI7VLQ .

talios commented 4 years ago

@stickycode did you have any suggested changes in the end at all? Was thinking of rolling a minor release using the new Groovy version, and updating the invoker plugin (and fixing the tests that breaks), but would be good to include something that fixes/improves tiles at the same time.

stickycode commented 4 years ago

Didn't quite get there yet, been busy doing some bounds and shifty

On Wed, 29 Jul 2020, 20:57 Mark Derricutt, notifications@github.com wrote:

@stickycode https://github.com/stickycode did you have any suggested changes in the end at all? Was thinking of rolling a minor release using the new Groovy version, and updating the invoker plugin (and fixing the tests that breaks), but would be good to include something that fixes/improves tiles at the same time.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/repaint-io/maven-tiles/issues/111#issuecomment-665534016, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG4GFBXWNLKF5JWSAJ7PFTR57QALANCNFSM4ONI7VLQ .

stickycode commented 4 years ago

Without looking at the code it seems like the transitives are added to one list at the end of the direct tiles.

How about depth first on the tile resolution. That would give an intuitive resolution or maybe make different weirdnesses