search

erlef / rebar3_hex

Rebar3 Hex library
Apache License 2.0
101 stars 49 forks source link

Revert "Add V7 publish options" #240

Closed tsloughter closed 3 years ago

tsloughter commented 3 years ago

Reverts erlef/rebar3_hex#235

Adding the options is fine but don't remove umbrella publishing. This is an important feature that without makes having multiple apps in a repo more painful. Maybe it needs some work? But removing it is not a good solution to any issues it may have.

starbelly commented 3 years ago

No, there was no problem (bug). The rationale behind removing umbrella support is :

That said, I can think of some improvements we can make if we keep it, but I'd be in favor of patching it back in vs reverting this entire commit.

tsloughter commented 3 years ago

If there wasn't a bug then it definitely shouldn't be removed because what is the "less to support" if it hasn't needed support? Simply not breaking it is the support needed?

As I mentioned in slack, this changes how rebar3 if explicitness requires chdir to the sub-app. In mix this "explicitness" is defined in mix.exs of the sub-app by referencing ../../_build and ../../mix.lock. In rebar3 we do not do this. rebar3 is built from the beginning to build projects with 1 to many OTP applications and sub-apps thus all share a _build and lock file without configuration, like in mix.

If now the publishing requires changing directory you will generate a new lock file, possibly different from the top level project lock file. That isn't adding explicitness.

Not sure what you mean about "hit y vs n", isn't that always the case? And there is a way to unpublish anyway.

starbelly commented 3 years ago

Fair points, very fair. As stated, I'd like to close this, patch the support back in and refactor the module such that there's two very distinct paths to go down (lib only path, and release path).

Edit :

Not sure what you mean about "hit y vs n", isn't that always the case? And there is a way to unpublish anyway.

Gets into a UX convo, but we can probably improve that, but would save that for its own issue.

starbelly commented 3 years ago

@tsloughter as discussed, closing this in favor of #241