liferay / liferay-theme-tasks

A set of tasks for building and deploying Liferay Portal themes.
18 stars 24 forks source link

Remove support for 6.2 themes #57

Closed robframpton closed 6 years ago

robframpton commented 7 years ago

Currently the theme tasks support both 6.2 and 7.0 themes, I would like to drop support for 6.2 and clean up the code related to it.

We probably shouldn't remove the upgrade task for 6.2 themes however, or find another way to deliver that functionality.

izaera commented 6 years ago

@Robert-Frampton I've been looking at how to do this task, and I think it should be pretty straightforward. Even the upgrade task could be deleted directly IMO, if we promote the next release to version 2.0, instead of continuing with 1.x series.

As I understand it (though I maybe wrong, of course) it makes sense to release a new major version each we release a new portal, and let old projects continue using the older theme-tasks until they upgrade the project.

If we do it that way, someone who had a 6.2 project, would still be able to call gulp upgrade as long as she doesn't use the new release. Then, after the updgrade, the same user could change her package.json and upgrade to theme-tasks 2.0, losing the upgrade functionality, but without worries as it would be no longer needed.

izaera commented 6 years ago

After talking to Chema, he says that themes' structure doesn't change so often, but only the content of the CSSs, HTMLs, and so on. However, my reasoning still applies.

But another doubt came to my mind: if you are upgrading, you should specify the source and target version which now doesn't seem to be necessary. I suppose we can get it from the metadata and upgrade just one step at a time but, in that case, it would be better not to remove any upgrade code. In fact, it doesn't make too much sense in my opinion, because you don't get much advantage and remove functionality from the tool.

If we always have old upgrades available then nobody needs to care about which version of the tool to use: you just always use the last and everything goes fine. On the contrary, if we remove upgrades, then you -as a user- have to find the version that supports your required upgrade, set it, install, and call upgrade. Then return to the latest.

Don't know if I'm explaining myself... 🤔

robframpton commented 6 years ago

Hey @izaera,

Yeah so the idea for the upgrade task is that it just upgrades your theme to the next version, so you would never need to specify what version you are upgrading to, as you said "one step at a time."

Currently the tool is set up in such a way that it would be easy to add a new upgrade task for 7.0 -> 7.1. You just need to add a 7.0 folder to the https://github.com/liferay/liferay-theme-tasks/tree/master/lib/upgrade directory and provide the upgrade.js file.

In regards to if we should remove it, we can technically remove support for building/deploying 6.2 themes in the next major version, but keep the upgrade task around for easier migration. Personally that's what I'm in favor of, I wouldn't want to make things so confusing for users. What are you leaning towards?

izaera commented 6 years ago

Oops, sorry, I messed up my head. Yes, we are on the same page: not removing the upgrade task. For some reason I started thinking that you wanted to remove it too 😬 .

robframpton commented 6 years ago

I think originally I was leaning that way, but I changed my mind :)

izaera commented 6 years ago

Still, if we remove support for older versions and somebody wants to create a theme for an older one, he would need to use an older version of the tasks. If he then finds a bug, we would have the "problem" of having to support two version lines (in favor of clearer code). Is that what we want?

izaera commented 6 years ago

The other alternative is to always force to use the latest version and support all versions of the portal there 🤔

robframpton commented 6 years ago

6.2 isn't officially supported in regards to these tools, so even if a 6.2 user runs into a problem, we shouldn't have to worry about it.

izaera commented 6 years ago

In that case I will start removing code and then I'll send you a PR. Thx :-)

yuchi commented 6 years ago

Here’re my usual 2¢.

There are a lot of companies that switched over to the node build process for older 6.2 projects.

Are you willing to release new minor versions of 1.x in order to fix problems on that side?

jbalsas commented 6 years ago

Hey @yuchi, 6.2 should be reaching EOL quite soon, so supporting creation of 6.2 themes is definitely not a priority at this point. We're keeping upgrade for 6.2 themes onward, and we're focusing on rolling out official support for this starting on 7.1 and moving back from there to 7.0, so I think our efforts are better directed towards those goals.

I've merged this into develop, so closing here as completed until we release!

yuchi commented 6 years ago

Sad but understandable. Worst case scenario we'll fork.