Closed Bug 1299448 Opened 8 years ago Closed 8 years ago

Figure out how to manage our source code and submit changes

Categories

(Web Compatibility :: Interventions, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: denschub, Assigned: denschub)

References

Details

Attachments

(1 file)

Creating this as a discussion bug for talking about how we can manage our source code and how to submit patches.
So, this turned out to be much more interesting than expected and we cannot just copy anothers project ideas here since e10srollout, flyweb and pocket are build in tree and they submit simple patches to mozilla-central.

tl;dr: We have two options:

* Building the extension into `webcompat-repo/build/`, symlink that folder to `mozilla-central/browser/extensions/webcompat` and generate patches by `git diffing` the build folder between tags or something.
* Having the build script write its files into `mozilla-central/browser/extensions/webcompat` and submit patches using hg.

The only system extension that's partly build outside of mozilla-central is pdfjs and they have a GitHub repository with the library and a build process in the github project to generate the pdf.js and update the sources in the tree with bugs like bug 1282095. They've build a job [0] that basically builds the extension and overwrites the files in a checked out mozilla-central copy.

Another option would be to have the current mozilla-central-sources cloned in our repository (the state we have right now), have a build process that builds the extension into that folder and get a local patch. For development, symlinking the build output directory over `browser/extensions/webcompat` could work. This was the initial idea.

I have to say I really like the idea of having a build job that simply throws its output into the checked out mozilla-central. While this requires everyone working with the extension to have a m-c clone, it would make generating patches much easier since we could simply use hg for that. Also, we do not need to take track of the source in mozilla-central, since we can simply overwrite the files in the extension folder and submit a patch. The only downside is that we need a way to figure out where m-c is located. pdf.js assumes it's inside a `mozcentral` directory inside the pdf.js repo, but I don't really like that. Maybe an environment variable containing the path is a good solution.

As it is much simpler, I like the idea of having our build task write the files into mozilla-central for the reasons mentioned above.

CC'ing Eric and Mike, but I'm gonna submit a PR with a basic build script hopefully later today, so I'll ask for official feedback there. However, feel free to jump in if you have strong opitions, questions or anything else.

[0]: https://github.com/mozilla/pdf.js/blob/master/make.js#L761
Assignee: nobody → dschubert
loop (aka Hello) was also a system add-on devved on GitHub: https://github.com/mozilla/loop/blob/master/docs/Developing.md

When I spoke with :Osmose initially about our gofaster add-on, he recommended doing our own thing in GitHub, then submitting a big diffpatch to m-c when we want to release a new "version". I think that's what you're describing -- so SGTM!

> Maybe an environment variable containing the path is a good solution.

Yeah, seems good. loop also used a EXPORT_MC_LOCATION envvar with a default (you can read more in the developing.md link above, but the general idea is the same).

(Also, if you want some idea sanity-checking, Osmose is a good person to ping since he's done this a few times before.)
Mike, true, I totally forget about loop for whatever reason. Thanks for the heads-up.

Eric, I will ask you for review now, so we both are aware of what is going on and we can work together. Mike will know what's up anyway. :) In the attached Pull Request, I've built a basic Jakefile (that's a... JavaScript Makefile) that allows exporting the sources into a checked out mozilla-central copy as well as running the changed addon-sources from there. As dicussed here, I think this is the nicest method and seeing that loop did the same confirmed that for me.

I decided to move the actual source files into a `src/` subdirectory. I've thought quite a bit about that since it looks somewhat weird, but I've had good experience with that in the past since it makes exporting the source files a bit easier (there are no project meta files like the README, .git directories, ... in it) and I never heard someone objecting. Obviously, we can discuss about that! :)

The Jakefile included in the PR provides basically two commands, `npm run jake export-mc` and `npm run jake run-mc`. The first one copies the files with some exceptions to the mozilla-central copy so it can get commited/exported/pushed to try, ... The second task exports the files and calls `./mach run` which should make testing easier.

The .eslintrc is stolen from mozilla/example-addon-repo and it matches the existing code style in our other repos, so this should be fine for everyone.

*cough* Sorry for the delay with this PR, I ended up continuing my work on the actual UA override mechanism far further than I probably should have - but the good thing is that this is almost done and ready to rebase after we have everything else figured out...

Feel free to ping me if you have questions.
Attachment #8788290 - Flags: feedback?(etsai)
Comment on attachment 8788290 [details] [review]
Create tools for exporting and running sources

Gnah, wrong flag.
Attachment #8788290 - Flags: feedback?(etsai) → review?(etsai)
Comment on attachment 8788290 [details] [review]
Create tools for exporting and running sources

I left comment on PR.
IMHO, we should provide both big diff to m-c for formal firefox release/testing and independent addon build for addon update[1]. Build with firefox solves first release problem, we just keep code on m-c sync with independent build. If the addon need to build with firefox, current implementation looks good to me.
 
Another question is we may frequently update rules, so this could be a live updated from some other server or within addon. The latter means we may takes time handle release addon, does this really happen to us?

[1]: https://wiki.mozilla.org/Firefox/Go_Faster/Releasing_an_add-on_mechanics
Attachment #8788290 - Flags: review?(etsai) → review+
Thanks Eric. You are right, I'll add a task to build an .xpi out of the sources as well. I'll ask for another review of that as soon as I am done.

> Another question is we may frequently update rules, so this could be a live updated from some other server or within addon. The latter means we may takes time handle release addon, does this really happen to us?

Our addon updates do not have to ride the trains, they can get updated using the addon updating mechanisms and those should be quite fast. I assume this is good enough for us, but if we run into issues with too slow updates, we can always think about a more complex way later.
Yeah, I think we'll have a better sense of how fast we're moving and how faster we want to be moving with some real experience. Good question though.
Comment on attachment 8788290 [details] [review]
Create tools for exporting and running sources

Eric, although you've technically already r+'d the build, I've added a step and some helpers for building the .xpi. Do you mind reviewing again, just to make sure I didn't miss anything?
Attachment #8788290 - Flags: review+ → review?(etsai)
Comment on attachment 8788290 [details] [review]
Create tools for exporting and running sources

Looks good to me, thanks Dennis
Attachment #8788290 - Flags: review?(etsai) → review+
Thanks Eric! So, basically, that's what we have:

1. Change sources in the webcompat-addon-repo
2. Run `npm run jake export-mc`
3. Commit the changes in m-c, upload patch to bugzilla, ...

So, as I've got two r+ from Eric and nobody seems to complain, I've merged the PR. Marking this as RESOLVED, since we now know how to generate patches!
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.