Closed Bug 1458668 Opened 6 years ago Closed 5 years ago

Create changelog for public API

Categories

(GeckoView :: General, enhancement, P2)

59 Branch
enhancement

Tracking

(geckoview64 wontfix, geckoview65 wontfix, firefox-esr52 wontfix, firefox-esr60 wontfix, firefox59 wontfix, firefox60 wontfix, firefox61 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

RESOLVED FIXED
mozilla66
Tracking Status
geckoview64 --- wontfix
geckoview65 --- wontfix
firefox-esr52 --- wontfix
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: snorp, Assigned: agi)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

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.
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()
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".
Assignee: nobody → snorp
Priority: -- → P2
Summary: Create change log for public API → Create changelog for public API
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?
Flags: needinfo?(s.kaspari)
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/
Flags: needinfo?(s.kaspari)
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
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?
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.
Assignee: snorp → agi
Blocks: 1502115
: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.
Flags: needinfo?(snorp)
(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.
Flags: needinfo?(snorp)
> 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.
Depends on: 1506267
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'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.
(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.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
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
Attachment #9026706 - Attachment description: Bug 1458668 - Enforce changelog for GeckoView; r?snorp! → Bug 1458668 - Enforce changelog for GeckoView; r=snorp!
Still need to add the content of CHANGELOG.md to our javadoc.
Keywords: leave-open
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4473b89c6603
Enforce changelog for GeckoView; 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
Jekyll Template for CHANGELOG.md: https://github.com/mozilla/geckoview/pull/3
Keywords: leave-open
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/30abf82080a3
Link CHANGELOG.md in javadoc. r=snorp,geckoview-reviewers
https://hg.mozilla.org/mozilla-central/rev/30abf82080a3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Product: Firefox for Android → GeckoView
Version: Firefox 59 → 59 Branch
65=wontfix because we don't need to uplift the changelog script.
Target Milestone: Firefox 66 → mozilla66
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: