-
Notifications
You must be signed in to change notification settings - Fork 1.2k
App Contribution
thyttan edited this page Sep 17, 2023
·
13 revisions
This page is still a work in progress
When contributing a new app, you should check out the Bangle.js tutorials - specifically http://www.espruino.com/Bangle.js+App+Loader
If you're a new contributor, please get stuck in. These items are a reference for maintainers, but even if you're not sure how to check everything below we're open to contributions and can tell you if anything needs changing. However if you submit a PR and do not respond to questions on it in a week or so then we may close that PR without merging.
- The CI tests for a PR complete successfully (the green tick or red cross for failure by a PR)
- The PR doesn't include changes to other apps that aren't mentioned
- The PR doesn't include temporary/accidentally committed files
- Compatibility with current stable firmware version (and maybe most recent?)
- Compatibility with dark/bright themes
- Useful/correct metadata
- storage files listed
- data files if settings are used
- supported hardware
- tags
- type (if a clock or not an app)
- If a new library is used in the repository
- Have a look at licenses
- See if it was already used in the repo and stored globally in the
modules
folder (eg suncalc) - Is it really needed? For instance should there be a new layout library added to the
modules
folder if it's undocumented and used by just a single app?
- Is something re-implemented when really existing functionality should be used.
- Alarms/timers should ideally use the
sched
library
- Alarms/timers should ideally use the
- Get another person (at first @gfwilliams) to get a view at the PR if:
- Some default for multiple apps is changed
- "Basic" apps like Android, Bootloader, Settings etc. are changed in a relevant way
- Screenshots (especially for clocks) - on Bangle.js 2 you can run
g.dump()
in the IDE console to get these (see https://github.com/espruino/BangleApps#screenshots) - A link to the new/updated app on your own copy of the App Loader in the opening comment of the PR. Make sure the branch used for the App Loader and the PR is the same.
- ((thyttan) is this too picky?) A commit ideally doesn't contain changes to more than one app.
- Is the version bumped?
- If the app utilizes pre-minified versions of the code (e.g.
app.min.js
orlib.min.js
) make sure that was also updated. - We're not fond of you refactoring existing apps to your personal style - best stick with the code style used for that particular app. And if for some reason you have to refactor it should be in a separate commit so we can see what actually changed.
- Avoid hacks to work around particular problems. If something is broken in some other part of Bangle.js then ideally we should raise a bug for that and fix it, rather than adding a hack to every app that wants to use that functionality.
- Should it be a fork? See below.
- Is a fork needed or should the original app just be modified?
- If the original app was very small, maybe a fork is better.
- Battery widgets for example are tiny, and there's no point doubling the size of the code and adding a settings file if there could just be a new app instead.
- If the original app is bigger (especially if it already has a settings page), could the change be implemented as a setting? Or if it's just a good feature/bugfix maybe it should just be added by default to the original app
- If the original app was very small, maybe a fork is better.
- Preference regarding how to merge (Create a merge commit/Squash and merge/Rebase and merge). I (thyttan) think the precedent is to use a merge commit?
- (Gordon) I think in general a rebase and merge is best. It seems for most PRs there aren't that many commits involved, but if there are absolutely loads then maybe squashing does make some sense
- (thyttan) I added a 'nice to have' saying a commit preferably only carries changes to one app. Is that too picky? Should it be more picky? should it be less of a 'nice to have' and more of a prerequisite for merge? I asked for this on a PR here.