Mail: Only use »make« for compiling

[jancborchardt]

Instead of the two-step process:

make install-composer-deps
make optimize-js

Can we make it so that simply running »make« does everything? It seems like that’s even already the case and we just need to change the readme?

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[ChristophWurst]

well, once we decided that 'make' should build the app store package. And that's what it still does. We should try to find a common procedure across all apps (e.g. calendar, contacts and news).

[ChristophWurst]

I'm not a big fan of the Makefile anyway because it abstracts the underlying technology away, although that is widely known and accepted in other web projects (composer, bower, npm).

[jancborchardt]

One of our main goals is to make it easy for people to contribute. Building the app store package is something only the maintainer does, and not often. So it could be »make app-store« or so.

make should ideally:

• install all the dependencies needed for building (this node and npm stuff especially which is a drag …)
• carry out all the commands
• if any command fails because of cached files or whatnot, clean them up

cc @Henni @irgendwie @georgehrke @raimund-schluessler @v1r0x @oparoz for other apps.

[ChristophWurst]

For newbies it might be too complicated to use the tools directly, but anybody who has experience with web development knows how to deal with npm and bower. The latter are probably confused more when they find the Makefile because that is very uncommon for this kind of software project (they are common for C/C++ and the like though).
But I see your point. If some1 volunteers to create this magic Makefile, I'm happy to review their PR 😉

[Quix0r]

GNUSocial uses a Makefile to build e.g. language files (gettext).

[ChristophWurst]

Well, translations is about all the Makefile does. https://git.gnu.io/gnu/gnu-social/blob/master/Makefile

[Quix0r]

Yupp. :-)

[jancborchardt]

anybody who has experience with web development knows how to deal with npm and bower

Sorry but this is not entirely true. They are all commands you need to remember, and there are varying degrees of »experience with web development«. And we also want to attract designers, and people who primarily work with HTML and CSS. They don’t necessarily know how to grunt build and npm install and all this stuff.

[oparoz]

I don't think the problem is the makefile. From the moment you need bower, npm, etc. It's game over for beginners. They need to be able to edit JS files and see the results right away.

[ChristophWurst]

Ok. So does anybody have a good solution for this?

Some people argue that the simplest way is to include everything in version control. This is great in regards to the steps needed to check out the app. However, this is likely to make PR reviewing and some version control actions (merge, rebase) a major pain.
Here's a simple example. I recently updated Marionette to version 3, which means I had to bump the bower dependency version _and_ adjust some code to remove deprecated/removed APIs. The original PR can be found in #81.
If we had all deps in git, the diff for reviewers would have looked like this: https://gist.github.com/ChristophWurst/1fddec730affdecbad715d7591e1cc93. Have fun finding the real code changes.
Another problem that would emerge is that we can no longer depend on stuff that is downloaded via git (e.g. bower deps that are fetched from GitHub) because git does not handle those well, as it treats them as submodules then. Read the composer docs for more info.

[jancborchardt]

Yeah, I mean npm (and node) are the real killer here. We also had major problems setting that up at the Ladies that FOSS event for the Contacts app – actually resorted to copying over my node_modules and other js/vendor and js/public folders to make it work. :\

I know that having the dependencies in git is a pain too, and it doesn’t seem like a good solution when they are updated very frequently. However in practice, how often are they updated? And what are the other options?

• composer
• git submodules
• make which gets the deps via curl (lol?)
• …

[ChristophWurst]

Just found this: https://packagist.org/packages/fxp/composer-asset-plugin. It scares me that so many people have downloaded it already. I wouldn't expect it to work flawlessly, but one could simply try and see if it works for our dependencies.

[ChristophWurst]

In addition I found out that we use npm mostly for CI stuff. We could actually use bower's require package to create the optimized js file. This means devs wouldn't have to install npm deps anymore. Therefore we'd only have to install composer and bower deps. Not perfect, but better than the current situation.

[oparoz]

If we had all deps in git, the diff for reviewers would have looked like this: https://gist.github.com/ChristophWurst/1fddec730affdecbad715d7591e1cc93. Have fun finding the real code changes.

I think this can be solved by making sure there is one commit to make the dependency changes and one with the real changes.

Another problem that would emerge is that we can no longer depend on stuff that is downloaded via git (e.g. bower deps that are fetched from GitHub)

True. One would have to run composer/bower himself to get everything configured for his environment.

[jancborchardt]

@ChristophWurst cool! So replacing npm with the bower stuff seems good. Is that something we can do in Contacts too @Henni @irgendwie?

Also, what do you think about @oparoz’s proposal with the separate commit for the dependency update? Seems good, or does it happen too often?

[ChristophWurst]

Also, what do you think about @oparoz’s proposal with the separate commit for the dependency update? Seems good, or does it happen too often?

Depends on how well GitHub handles those big diffs. I guess we'd then have to review those PRs on a commit basis then instead of the full diff.

[jancborchardt]

@ChristophWurst any further idea here btw – how do we want to proceed? :) I’d like to have the Mail app be as friendly & easy for new contributors as possible.

[ChristophWurst]

In addition I found out that we use npm mostly for CI stuff. We could actually use bower's require package to create the optimized js file. This means devs wouldn't have to install npm deps anymore. Therefore we'd only have to install composer and bower deps. Not perfect, but better than the current situation.

Tried that but doesn't work. The bower package does not ship the require optimizer. I will try to include bower & npm in git. But we should do this carefully and step-by-step. I'm against adding composer deps to git thought as the composer devs have added the warning to their documentation for a reason. We shouldn't ignore that.

[jancborchardt]

Actually ran into this just now when we wanted to set up the Mail app in @julieannenoying’s dev environment (with Windows + Ubuntu VM for server). In Git Bash on Windows the make command is missing, and the Ubuntu VM doesn’t have internet connection because it needs to use Host-only internet so it can work as a server …
(Now doing it via the VM … npm is of course also not installed in default Ubuntu server. ;) You see it’s not easy at all.)

Would really be cool if we can solve this so not only highly technical people can contribute. ;) Then we can grow the community more.

[jancborchardt]

Aaand this is where we finally gave up :(

(The nextcloud folder is on the Windows host, mounted. Only the VM has Ubuntu with make. Everything else worked – it’s mounted read/write but I guess something can easily not work with all the stuff like composer and npm. :\

[ChristophWurst]

I tried to add npm deps to the repo …

17,739 changed files with 1,879,688 additions and 12 deletions.

Since that will unnecessarily increase the size of this repo and npm deps are only needed for production builds, I'm against adding it.

[jancborchardt]

What’s the actual size increase on the repo though? And is there any other step we can take on this then? :)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.