Stop merging/unmerging profiles and parts of profiles randomly

[AdamClements]

simply encourage profiles which don't derive from base or user and have an :android-config in your profiles for setting the sdk path and any injections or dependencies which you want to include in all your android projects.

This also allows the :android :build-type :debug or :release option, rather than examining which profiles were included, this is far more robust and allows you to have multiple release and debug profiles. Generally this is more predictable and in line with the way that leiningen profiles are supposed to be used.

I have left the doall and release tasks for now for back compatibility and quick-start, but more and more they should be replaced my profile inheritance and simple aliases in the future.

Doall and release only pull in :android-dev and :android-release respectively, and both pull in :android-config which is expected to be found in your profiles.clj, these should be minimal tweaks to configuration and remove a lot of headaches and potential pitfalls.

[alexander-yakushev]

Wow, I certainly expected a lot more code to perform this transition :). Good work, thank you.

Minor details follow. Is there any particular reason to name the base profile :android-config and not :android?

[AdamClements]

I didn't like writing {:android {:android {:sdk-path ""}}}, it could confuse things and people might end up not nesting it appropriately.

[alexander-yakushev]

Right, my bad. I confused the two.

It seems that :android-dev and :android-config are only merged on doall task. What about doing manual lein droid build and others? Can we merge them by default with only exception being in the release mode? Or what is the better way to do this? I understand that you want to provide the correct profile-driven approach, but typing lein with-profiles android-config,android-dev droid build doesn't seem to be a pinnacle of usability. Can there be a compromise of some sort?

[AdamClements]

No, the whole point of this pull request is that we don't merge profiles unnecessarily, allowing the user to have a custom setup if they so wish, the doall and release tasks are simply for quick-start and convenience, in a future pull request I want to change them to :alias {"droid-doall" ["with-profile" "android-dev,android-config" ["droid" "build"] ["droid" "apk"] ["droid" "deploy"]]} and put that alias in the sample config, so people can easily see how to manually perform the steps, activate different profiles if they wish or manually enter all the steps they want to perform and make new aliases for tasks they want to do (without having to look into the lein-droid source code).

[alexander-yakushev]

I see the point of not messing with profiles inside the plugin, then can we, say, inherit from :android-config and :android-dev profile inside the :dev profile? Like:

:profiles {:dev [:android-config :android-dev
                 { ... }]}

[AdamClements]

Exactly right, we wouldn't need to put both profiles in the with-profile if they have profiles set up to inherit from the appropriate things, so perhaps updating the sample project is a better way of doing this.

As an aside, talking about the sample project, with the android manifest changes, there's now nothing stopping us from making a lein-new template and removing the need for having lein droid new and the lein-droid plugin in profiles.clj at all because we no longer need to prompt the user for project names and namespaces etc, they can all be part of the project.clj and obvious to change.

We just need a lein-droid/lein-template and people will be able to do "lein new lein-droid" with no configuration necessary and a sample project of the latest version.

[alexander-yakushev]

First things first. Changing the sample project and the template would be perfect to do in this pull request so that the plugin stays consistent. Next, I envision this scheme of dependency:

1. :android-dev and :android-release depend on :android-base (I think it is a better name than :android-config as the latter is not semantically tied to dev or release, while the former goes well with Leiningen's own :base profile).
2. :dev profile in the project depends on :android-base and :android-dev. Redundancy with base can be useful to tell the user what is the preferred name for the baseline android profile.
3. In the same way :release depends on :android-base and :android-release.

Next, we have to make the final decision about :user profile. Either we don't use it — then we have to explicitly exclude it from our profile dependency list (does Leiningen allow that?). Or we use it and somehow wipe the :dependencies and :repl-options that could be there. I personally don't mind which one to use — since the :android-base can contain all the options that :user contained before. However, if we explicitly exclude :user thus allowing the user to turn it back on, this might again break the setup with unsupported dependencies from the :user profile. So on top of explicit exclusion some kind of warning should be printed if :user is enabled anyway.

Finally, about the template. If you remember our talk on IRC, I don't want to replace droid new with new droid until there's a way to specify the template version. I asked on Leiningen's mailing list about it and people agreed that could be useful as long as I write it myself. So I'd like to put off this change until there's better foundation for it.

[AdamClements]

What I've called :android-config is not equivalent to leiningen's :base profile, it's equivalent to the :user profile and lives in .lein/profiles.clj

The equivalent to the :base profile would be if we added all the default value to an :android-base profile which you could choose to inherit from and which :android-dev and :android-release would inherit from in the sample, rather than at the moment where the function (android-parameters) gets called on the project.

One of the major benefits of this change is that it doesn't use the :user profile by default, which is why we no longer need to unmerge things arbitrarily. I am dead set against manually unmerging things from the user profile, far better to have an android device specific user profile android-config (or call it android-user if you prefer to highlight the parallel), this way you don't end up with the situation where you want a dependency in one project that isn't supported in another.

When you specify a profile, none of the default [:base :user :dev ...] ones are loaded. I don't see the point of specifying a :dev and :release profile when you could just specify :android-dev and :android-release to derive from :android-user/:android-base, what's your reasoning behind that?

We definitely shouldn't warn about using :user as that should be a perfectly reasonable thing to do if you're running the same code on your desktop that you will later run on your phone. We could even offer an example of loading with the stub android classes so you can get autocompletions and repl development without needing to be attached to the phone. This will also allow most standard test setups to compile and work, which is very important.

I have no problem with allowing both methods for the template.

[alexander-yakushev]

OK, :android-config might not be the mirror of :base, but :android-base better reflects its relation to dev and release than config. :android-user is actually quite confusing to me, so I still vote for android-base.

When you say that method does not use :user by default you imply that lein-droid should be always executed with with-profile ... suffix. It is first cumbersome, second it doesn't tell if the exclusion of :user is intentional or incidental. But it is possible to keep things easy with profiles only, that's what :default profile is for. It can be stuffed with :base (if it is needed, I'm not sure what Leinigen keeps in that profile), :android-base, :android-dev, intentionally omitting :user, and therefore the user won't have to specify all the profiles for his debug builds. And he still sees all cards on the table and is able to modify the profile inheritance logic to what fits him best.

And yes, of course there is a point in specifying :dev and :release profiles since they are specified on per-project level whereas :android-dev and :android-release are defined in profiles.clj. This is a better solution than to give them the same names in both places, see https://github.com/technomancy/leiningen/blob/stable/doc/PROFILES.md#merging

[alexander-yakushev]

My overall point is that in order to adopt this transition we have to:

1. Preserve the ability to run any lein-droid subtasks without specifying profiles for the release build.
2. Run any release-mode subtasks with lein with-profile release droid ...
3. All this should be achieved with profiles, no plugin involved; so that any user can modify his project.clj and profiles.clj to change the default workflow to whichever he prefers.

[AdamClements]

Okay, now I'm properly confused, why do you want android-dev and android-release in your profiles.clj? You only need the one profile (what I've called android-config) in your profiles.clj, the others I've been talking about live in your project.clj on a per project level.

And you wouldn't manually run each project with-profile, we'd provide aliases which included sensible default profiles, but you would have the capability to change profiles if you wanted to (a use case which I already have), and the inclusion/exclusion of :user is completely well defined in leiningen's profiles, if you specify a profile it loads the profiles you specify, if you don't specify any, it loads a default set, which assume you are developing on a jvm on a pc.

[AdamClements]

Perhaps it is easier to wait until I update the sample project with these changes, so you can see how it all fits together

[alexander-yakushev]

My usecase where I required android-dev profile in profiles.clj was adding a list of dev-dependencies that only Android projects depend upon (actually, it is now only cider-nrepl dependency). As for :android-release profile, it can contain the path to your release keystore (since it differs from setup to setup, it makes more sense to put it in profiles.clj rather than in the project).

I agree that having an initial rewrite of the sample project will make it easier for us to polish it later.

[AdamClements]

cider-nrepl, perhaps, but if your whole team is using cider, you probably want that in your project.clj anyway. There's nothing to stop you defining your own if you find that's a problem. I thought lein-droid had its own completion thing anyway?

As for the keys, that can just live in the same config profile alongside your sdk path - that was the logic of checking for dev mode before checking for release keys in my previous pull request. If you're in dev mode it won't use it anyway.

[alexander-yakushev]

that was the logic of checking for dev mode before checking for release keys

I'll be damned if it's not modifying profile behavior from the plugin:). Just kidding, we already agreed on that.

I thought lein-droid had its own completion thing anyway?

Yes, Compliment, but now CIDER migrated to Compliment as its completion backend. In any case, be it cider-nrepl or compliment, either looks better in profiles.clj rather than in project.

but if your whole team is using cider, you probably want that in your project.clj anyway. There's nothing to stop you defining your own

Exactly. Inheriting :dev from :android-dev (which is supposed to be in profiles.clj) won't magically create the latter, but gives a hint of a preferable name for it if you need it at some point.

[AdamClements]

Hmm, not sure I like that. We can put in in the wiki if you like, but I don't like the warning no profile found that you get currently, leaves the user with the impression they've done something wrong, loses some of their trust that it's set up appropriately

[AdamClements]

If you try and include a profile which isn't defined anywhere, when you build, the output is covered in "Warning, no profile :release found"

[alexander-yakushev]

Yes, just putting it on the wiki seems like a better solution.

So, finally, is the reason to name profiles inside project.clj like :android-dev instead of :dev is having multi-platform (JVM and Android) projects?

[AdamClements]

Exactly, in my project I already have :desktop-dev, :android-dev and :ios-dev ... etc. so this seemed sensible to be clear

[alexander-yakushev]

While this is a good solution, I doubt it is required in the default template. By the moment a user turns to writing multiplatform applications they should already be able to manage his profiles without the help of the plugin. In the sample project though a complex example with many profiles may be OK.

[AdamClements]

Mainly, it's so it doesn't get confused with the built-in dev and release profiles which get activated as part of the default profile list, plus I have seen people using the :dev profile inside their profiles.clj to do injections and so on. Much better to be 100% clear, this is an android profile, for running on android devices.

[alexander-yakushev]

Makes sense. I guess I agree with everything now, except for :android-config name. I'll wait until you modify the sample project/template to see how it looks in the end.

[AdamClements]

I've simplified the top level droid.clj and made sure it works with release and doall in their current forms, pulling in the appropriate profiles android-dev and android-config. Changing .lein/profiles.clj should still be the only change that people need to make to update to this version.

The doall and release tasks can be totally replaced by aliases if you want, have left them in the same place for compatibility.

We could also remove a whole chunk of what manifest.clj is doing and replace it with the template stuff instead, and instead of reading out of the xml file put all those defaults into the project map and let the user override them from there. Haven't touched that in this PR though, it can happen later.

[alexander-yakushev]

OK, looks great. I'd be grateful if you rewrote your commits so there are no repetitive changes of functions (like changing and fixing doall) and we are good to merge. There are a few more things I'd like to see here, but I can take on them myself from this point.

[AdamClements]

Done

[alexander-yakushev]

Merged, thank you!