Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly allow custom keys in the .app file #1793

Closed
wants to merge 1 commit into from
Closed

Explicitly allow custom keys in the .app file #1793

wants to merge 1 commit into from

Conversation

fxn
Copy link
Contributor

@fxn fxn commented Apr 20, 2018

This is really a question in the shape of a patch :).

The documentation in http://erlang.org/doc/man/app.html suggests to me that the property list is the one specified there, with those specified keys. But I have seen that some applications include custom keys, see for example hackney.app.src, or poolboy.app.src, which contain keys maintainers, licenses, and links.

Those particular keys are there because they are processed by rebar3_hex.

The question for someone generating application resource files is: is that technically valid and thus the proposed patch says something true? or it isn't, the patch is not good, and rebar3_hex should revise that contract and move that metadata somewhere else?

@okeuday
Copy link
Contributor

okeuday commented Apr 20, 2018

reltool has many warnings when it sees custom keys. My view is that the keys should become standard and that reltool should expect them.

@sirihansen
Copy link
Contributor

@tsloughter Could you please explain the use of the maintainers, licenses and links keys in rebar3_hex, and if the .app file is the only possible place to put them? Or could rebar.config be an alternative? Just so I get a better picture of the needs. Thanks!

@tsloughter
Copy link
Contributor

@sirihansen the idea was that these are specific to the application and not the build tool. It was a way to not have it be tied to rebar3 (allowing for it to be used by other programs that would publish to hex).

I did want to move it to rebar.config if .app.src was gotten rid of and instead the .app was generated from content in rebar.config. Because of umbrella projects with multiple apps we can't simply list metadata in rebar.config and know who it applies to, so it made sense to simply tie it directly to the application. If the application were defined in rebar.config like:

{apps, [{application, Name, [...]}]}

the issue would be moot.

@sirihansen
Copy link
Contributor

If we were to add one named key, under which you are allowed to insert custom sub-keys, would that solve the problem? If so, we would possibly want to add something like 'application:get_custom_key/2' for convenience also.

@tsloughter
Copy link
Contributor

Sure. Though since modules already have 'author' it wouldn't be that out of place to have something similar for the application as a whole, right? But I'm fine with a custom keys entry as well.

@sirihansen
Copy link
Contributor

I'm really sorry for the delay on this. We do not want an explicit 'author' key, but if you want to add a key named 'custom', which points to a key-value list we will accept the PR. Please remember to add test and documentation.

@sirihansen sirihansen added team:VM Assigned to OTP team VM waiting waiting for changes/input from author and removed team:VM Assigned to OTP team VM labels Dec 20, 2018
@fxn
Copy link
Contributor Author

fxn commented Dec 20, 2018

Perfect, I think this PR can be closed then.

Just not to be in a deadlock... @tsloughter would you work on that patch to closely coordinate with rebar3_hex? Could try to have a stab at it otherwise. In any case, I could try to revise resource file generation by Mix once a concrete solution is accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting waiting for changes/input from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants