search

SAP / commerce-gradle-plugin

Gradle plugins for the full development lifecycle of a SAP Commerce project
Apache License 2.0
33 stars 12 forks source link

Task/issue30 sparse bootstrap #43

Closed iccaprar closed 1 year ago

mpern commented 1 year ago

Thank you for the PR! On a first cursory glance I don't see any major issues. I really appreciate the additional tests and the updated 'dummy platform'

I will review the PR by the and oft this week (CW 7)

iccaprar commented 1 year ago

Thank you for the PR! On a first cursory glance I don't see any major issues. I really appreciate the additional tests and the updated 'dummy platform'

I will review the PR by the and oft this week (CW 7)

It would be also good to test it on windows, I am curious of the performance there.

iccaprar commented 1 year ago

In addition to the comments I’d suggest to remove the silly copyright notices from hybris or sap in the dummy extension files ;)

I will clean the copyright from the demo repos, thought we want them :D

iccaprar commented 1 year ago

@mpern - I am not so sure if the usage of Path in relativeLocation will generate any issues on Windows, this is why I avoided it so I am able to strictly control it as a unix like path. Unfortunately I do not have a Windows environment to test that, so I would prefer to create a new task so someone can try to change it later (maybe I will find some time to setup the project on a Windows machine in the next weeks, but cannot promise anything).

Let me take a look at the other comments and update the code, should not be a big problem. Will do it until Monday.

mpern commented 1 year ago

Path in relativeLocation will generate any issues on Windows,

Windows compatibility exactly the reason I avoided using Strings and use Paths everywhere 😁

iccaprar commented 1 year ago

Yeah... The good thing is that windows accepts without issues unix paths with /. We already tried the plugin in the team with Windows and it works fine (slower only...).

Output for windows:

Unpacking platform in sparse mode
Loaded extensions information from project custom folder in 160 ms
Loaded extensions information from hybrisPlatform dependencies in 10539 ms
Loaded all needed extensions from localextensions.xml in 2 ms
Loaded existing extensions information from project folder in 107 ms
Some needed SAP Commerce Suite extensions are missing, copying them
mpern commented 1 year ago

Yeah, as I've said it's mostly just me preferring Paths and trying to be consistent.

You already contributed so much valuable effort, I'm perfectly fine if you tell me that you will not invest more time and someone else should do it.

iccaprar commented 1 year ago

No worries, let's create a new task to check this separately and I hope I find some time in the next weeks to do it (and maybe also remove the absolute path from the Extension with it). Just cannot promise it this week and I would really want the plugin to be used by our project team soon :) (some already took the branch from my own repo fork and added the plugin in the project build... ).