1458668 - Create changelog for public API

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P2)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

We need to have API changes clearly documented somewhere. Maybe we can include this into our javadocs somehow? A separate file (or files?) could work too, but it needs to be very easy for app developers to view this.

We would ideally have some tooling such that we could detect an API change at build time and fail the build if the "api changelog" had not been updated.

The Android SDK annotates methods with the API version that first included the method. Because they never change or remove public API that's basically all they need. We are not in the same position.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

One idea for this would be to create a hash of the public classes/methods/fields in org.mozilla.geckoview for each build. We would then look for this hash in the changelog file. If not found, the API has changed and you must document the change in the changelog file and include the hash. I believe this should be relatively easy to wire up in gradle, though I could be wrong there.

Example change log:

GeckoView 61.0:

- (a2a9ff52) Renamed GeckoSession.fooBar() to GeckoSession.baz()
- (b1491ffe) Added GeckoSession.fooBar()

Comment 2

[Joe Walker [:jwalker]]

Is there a danger that it could give a false sense of security? API compat is about more than just the veneer. Not saying it's necessarily a bad idea, but we'd need to make sure people didn't use that as the only answer to the question "did I break backwards compat".

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Sebastian, it seems like you would want something similar to this for Android Components. Do you have any interest in the system described in comment #1? Or maybe a better idea?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

Definitely interested!

Currently we are generating API reference docs with dokka. For every release I would like to generate an API Differences Report like Android does:
https://developer.android.com/sdk/api_diff/27/changes

However I do not know how yet. Just filed this in our repository too:
https://github.com/mozilla-mobile/android-components/issues/667

So far we are writing a changelog for every release:
https://mozilla-mobile.github.io/android-components/changelog/

Comment 5

[[ex-Mozilla] Agi Sferro | :agi]

I was looking into how android handles this, and it looks like they have a `api` file that they use to define what makes up the API [0] and they use it to check that the api did not change.

If the API changes the developer has to update the api file running a script and submit the change for review.

We could use the same system for geckoview and that will give us the API diff too as you can just diff the API file. With this:

- We ensure no accidental changes get through without proper review
- We have a clear log of API changes
- We can potentially generate a stub from the api file (:snorp suggested this)
- We can probably create a gradle plugin with this (also :snorp)

[0]: https://android.googlesource.com/platform/frameworks/base/+/master/api/current.txt

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

One thing I think we still want to add is a human-written description of the API change. This would include suggested migration strategies, caveats, etc. If we have the build integration for the api file described in comment #5, it may not be necessary to have strict enforcement of the descriptions as laid out in comment #1. A markdown file that gets pasted into the package documentation might be enough?

Comment 7

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1458668 - Create gradle plugin apilint. r=snorp,nalexander

This gradle plugin adds several tasks for API managment:

- apiLint<variant name>:
    This task will check that the current API is backwards compatible with the
    api specified in a file (by default called 'api.txt').  This task also
    checks for various lints as specified in //apilint/src/main/resources/apilint.py.

- apiDiff<variant name>:
    This task will print a diff between the current API and the local API

- apiGenerate<variant name>:
    This task will generate the current API file and store it in the build/
    folder

- apiLintHelp<variant name>:
    This task will print an help text whenever an API change is detected

- apiUpdateFile<variant name>:
    This task will update the current API file from the local API file whenever
    an API change is ready.

Comment 8

[[ex-Mozilla] Agi Sferro | :agi]

:snorp, regarding Comment #6, would `hg log <api file>` be enough as an API changelog or do you think we need something better? We need to make sure we write good commit messages but that's regardless of what method we use I think.

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Agi [:agi] from comment #8)
> :snorp, regarding Comment #6, would `hg log <api file>` be enough as an API
> changelog or do you think we need something better? We need to make sure we
> write good commit messages but that's regardless of what method we use I
> think.

Are you proposing a script of some kind that would copy the commit messages to a more visible location? Otherwise, I don't think commit messages have enough visibility to fill this role.

IMO, commit messages and the proposed API changelog serve different purposes and address different audiences. A commit message is mostly aimed at other folks collaborating on the project, where the API changelog is focused on consumers. They may often have overlapping information, but I think it's worth keeping them separate. We'd probably need to do a bunch of massaging of the commit messages anyway, if you're thinking along the lines of the script described above.

Comment 10

[[ex-Mozilla] Agi Sferro | :agi]

> Are you proposing a script of some kind that would copy the commit messages to a more visible location? Otherwise, I don't think commit messages have enough visibility to fill this role.
> 
> IMO, commit messages and the proposed API changelog serve different purposes and address different audiences. A commit message is mostly aimed at other folks collaborating on the project, where the API changelog is focused on consumers

I was proposing that but the more I think about it the more a `changelog.md` file that we can update with changes sound the best way to go. We can easily tie it to the sha1 or similar of the `api.txt` file like you were describing in Comment #1.

Comment 11

[[ex-Mozilla] Agi Sferro | :agi]

Thinking out loud about `CHANGELOG.md`

How do we want to version it? per gecko release? I was thinking something like

`CHANGELOG.md`
```
# v66
- Moved `blah()` to `foo()`
- Added `bar()`
# v65
- Removed `ohgodwhy()`
- ...
```

and somehow the script can make sure that the version is there (or not..).

We could also have api subversions like `65.0`, `65.1` that we bump every time we update the api in a specific gecko version but I'm not sure if that would be helpful or not, as I would expect embedders to just update from one major version to the other.

Comment 12

[[ex-Mozilla] Agi Sferro | :agi]

I'm also trying to figure out the best way to tie the CHANGELOG.md file to the sha of the api.txt file. 

I'm leaning towards having a field in the markdown which identifies the SHA of the api.txt file and needs to be updated every time the api file changes, something like:

```
[api-version]: 55895c49f55073d82d977cb74ec1d3a71ae4b25f
```

Or any equivalent syntax that makes this field hidden from the output.

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Agi [:agi] from comment #12)
> I'm also trying to figure out the best way to tie the CHANGELOG.md file to
> the sha of the api.txt file. 
> 
> I'm leaning towards having a field in the markdown which identifies the SHA
> of the api.txt file and needs to be updated every time the api file changes,
> something like:
> 
> ```
> [api-version]: 55895c49f55073d82d977cb74ec1d3a71ae4b25f
> ```
> 
> Or any equivalent syntax that makes this field hidden from the output.

Yeah, something like that sounds fine to me.

(In reply to Agi [:agi] from comment #11)
> Thinking out loud about `CHANGELOG.md`
> 
> How do we want to version it? per gecko release? I was thinking something
> like
> 
> `CHANGELOG.md`
> ```
> # v66
> - Moved `blah()` to `foo()`
> - Added `bar()`
> # v65
> - Removed `ohgodwhy()`
> - ...
> ```
> 
> and somehow the script can make sure that the version is there (or not..).
> 
> We could also have api subversions like `65.0`, `65.1` that we bump every
> time we update the api in a specific gecko version but I'm not sure if that
> would be helpful or not, as I would expect embedders to just update from one
> major version to the other.

I think versioning by major version should be ok. It's highly unlikely that we're going to put a new API into a dot release.

As for adding the version headers, I think we can just do that by hand when we need to add the first SHA + description for that version.

Comment 14

[[ex-Mozilla] Agi Sferro | :agi]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #13)

> I think versioning by major version should be ok. It's highly unlikely that
> we're going to put a new API into a dot release.
> 
> As for adding the version headers, I think we can just do that by hand when
> we need to add the first SHA + description for that version.

Sounds good. I implemented this in https://github.com/mozilla-mobile/gradle-apilint/pull/28, I'll bump the apilint version in GeckoView once that is merged.

Comment 15

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1458668 - Enforce changelog for GeckoView; r=snorp!

This patch integrates with apilint changelog feature to ensure we update the
changelog file every time the api changes.

It also adds a high level overview of the changes in v65.

See also: https://github.com/mozilla-mobile/gradle-apilint#changelog

Comment 16

[[ex-Mozilla] Agi Sferro | :agi]

Still need to add the content of CHANGELOG.md to our javadoc.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4473b89c6603
Enforce changelog for GeckoView; r=snorp

Comment 18

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4473b89c6603

Comment 19

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: Bug 1458668 - Link CHANGELOG.md in javadoc. r?snorp!

This moves the CHANGELOG.md file to a /doc-files folder that gets picked up by
javadoc.

Our javadoc files are hosted on a github.io page which will render the markdown
file with the geckoview profile.

Depends on D13883

Comment 20

[[ex-Mozilla] Agi Sferro | :agi]

Jekyll Template for CHANGELOG.md: https://github.com/mozilla/geckoview/pull/3

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30abf82080a3
Link CHANGELOG.md in javadoc. r=snorp,geckoview-reviewers

Comment 22

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/30abf82080a3

Comment 23

[Chris Peterson [:cpeterson]]

65=wontfix because we don't need to uplift the changelog script.