README for Thunderbird (comm repository)
Categories
(Thunderbird :: General, task)
Tracking
(Not tracked)
People
(Reporter: ryan, Assigned: ryan, Mentored)
Details
Attachments
(1 file, 7 obsolete files)
11.77 KB,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
We should have a README for the comm repository (aka Thunderbird) to help provide some guidance to those who pull down the source code.
The README should accomplish the following:
- Explain the Mozilla Codebase
- Teach how to build Thunderbird
- Teach how to run your build
- Point to how to contribute
- Explain how to submit a patch
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
Patch for the README, please review.
Comment 2•4 years ago
|
||
Comment on attachment 9061044 [details] [diff] [review] readme-for-thunderbird.patch Review of attachment 9061044 [details] [diff] [review]: ----------------------------------------------------------------- Could you at the same time remove the readme.html It's a good start. Some comments below Not sure if we need to mention SeaMonkey somewhere... ::: README.md @@ +1,2 @@ > +# Thunderbird > +Thunderbird an powerful and customizable open source Email client with lots of users. It is based on the Mozilla platform that Firefox is also built on. Thunderbird *is* ... Why Email with capital? Maybe "It is based on the same platform that Firefox is using." @@ +10,5 @@ > +In order to be able to build Thunderbird - you will need the mozilla-central repository as well as the comm-central repository (where this README lives). Check out our [Getting Started documentation](https://developer.thunderbird.net/the-basics/getting-started) for instructions on how and where to get the source code. > + > +### mozilla-central vs. comm-central > + > +Mozilla-central will build Firefox without the comm-central repo present and a few options set \(detailed below). Mozilla-central is the Firefox codebase and comm-central features the additions that turn Firefox into Thunderbird. The mozilla-central repostitory contains the Firefox codebase and all of the platform code. The comm-central repository is added as a subdirectory "comm/" under mozilla-central. This contains the code for Thunderbird. @@ +24,5 @@ > +- [macOS Build Prerequisites](https://developer.thunderbird.net/the-basics/building-thunderbird/macos-build-prerequisites) > + > +### Build Configuration > + > +To build Thunderbird, you need to create a file named `mozconfig` to the root directory of the mozilla-central checkout that contains the option `comm\mail` enabled. You can create a file with this line by doing this in the `source/` directory: comm/mail @@ +40,5 @@ > +_Each of these ac\_add\_options entries needs to be on its own line._ > + > +For more on configuration options, see the page [Configuring build options](https://developer.mozilla.org/en/Configuring_Build_Options). Note that if you use an MOZ\_OBJDIR it cannot be a sibling folder to your source directory. Use an absolute path to be sure! > + > +### Build the Lightning Calendar when building Thunderbird some trailing space here @@ +42,5 @@ > +For more on configuration options, see the page [Configuring build options](https://developer.mozilla.org/en/Configuring_Build_Options). Note that if you use an MOZ\_OBJDIR it cannot be a sibling folder to your source directory. Use an absolute path to be sure! > + > +### Build the Lightning Calendar when building Thunderbird > + > +Add the following line to your `mozconfig` file: .mozconfig @@ +74,5 @@ > +```text > +./mach run > +``` > + > +There are various command line parameters you can add, e.g. to specify a profile. , such as: -no-remote -P testing --purgecaches @@ +156,5 @@ > +If you want to ask questions about how to hack on Thunderbird, the IRC channel you want to join is [\#maildev on irc.mozilla.org](irc://irc.mozilla.org/maildev). > + > +### Report a Bug and Request Features > + > +### [Bugzilla](https://bugzilla.mozilla.org) https://bugzilla.mozilla.org/enter_bug.cgi?product=Thunderbird @@ +158,5 @@ > +### Report a Bug and Request Features > + > +### [Bugzilla](https://bugzilla.mozilla.org) > + > +Thunderbird uses Mozilla's Bugzilla platform to report and track bugs. The site can also be used to generate enhancement bugs, which can be used for feature requests. If you want to become a contributor to Thunderbird, you will need an account on Bugzilla as you will submit patches through this platform. I think bugzilla is it's own project, so perhaps just say "Thunderbird uses bugzilla to reporting and tracking bugs as well as enhancement requests." The " as you will submit patches through this platform" part I think we can drop (only half true with phab coming, and it's not a platform but a tool. @@ +162,5 @@ > +Thunderbird uses Mozilla's Bugzilla platform to report and track bugs. The site can also be used to generate enhancement bugs, which can be used for feature requests. If you want to become a contributor to Thunderbird, you will need an account on Bugzilla as you will submit patches through this platform. > + > +### Fixing a Bug and Submitting Patches > + > +All the issues, bugs, work in progress patches, or updates related to Thunderbird, are listed on [BugZilla](https://bugzilla.mozilla.org), and are properly organized per **Product**, **Component**, and **Status**. I'd add the link directly listing recent Thunderbird bugs @@ +170,5 @@ > +Creating an account is necessary in order to submit patches, leave comments, and interact with any other aspect of BugZilla. If you're currently using an `IRC` username in the `#maildev` channel, we recommend saving your profile name with the current format `Firstname Lastname (:username)` in order to be easily searchable and allow the Thunderbird team to offer better support. > + > +#### Find a Bug > + > +Use the [Advanced Search](https://bugzilla.mozilla.org/query.cgi?format=advanced) section to find bugs you want to take care of, and be sure that the bug doesn't currently have any user listed as _Assignee_ and the _Status_ is set to `NEW`. Here too add the direct link for thunderbird, so the search is restricted to Thunderbird, Mailnews, Calendar and Chat @@ +172,5 @@ > +#### Find a Bug > + > +Use the [Advanced Search](https://bugzilla.mozilla.org/query.cgi?format=advanced) section to find bugs you want to take care of, and be sure that the bug doesn't currently have any user listed as _Assignee_ and the _Status_ is set to `NEW`. > + > +#### Search for code references some of the headers are capitalized, some not, please make it consistant @@ +175,5 @@ > + > +#### Search for code references > + > +Making sense of the **Thunderbird** source code, and knowing where to look, will take some time. The code base is pretty big and if you never worked with `XBL` or `Custom Elements` it can be overwhelming at first. We recommend using our code search engine, [SearchFox](https://searchfox.org/comm-central/source/), to inspect the source code and find snippets and references to help you out while investigating a bug. > + Searchfox (lower case F) @@ +190,5 @@ > +0. Run `hg pull` to download the latest changes from the remote repository. > +1. Run `hg update tip` to update your current working directory to the most recent changeset that you just downloaded. > +1. Run the command `hg bookmark some-bookmark-name` to create a bookmark that points to the current changeset. Usually the name of the bookmark is related to the bug or feature you are working on. > +2. Run `hg bookmarks` to list all of your bookmarks and see which one is currently active. The one you just created should now be active. > +2. You can run `hg log` to see which bookmarks point to which changesets. Or you may prefer using a GUI tool like [TortoiseHG](https://tortoisehg.bitbucket.io/) that provides a more graphical overview. Not sure I'd want to promote TortoiseHG. I doubt contributors really should be using it. @@ +205,5 @@ > +Run `hg export --rev feature-A > my-feature-a-file.patch` to create a patch file. The `--rev feature-A` part indicates which revision (changeset) you want, in this case the changeset that the bookmark `feature-A` points to. > + > +#### Upload a Patch > + > +Open your `patch-name.patch` file in your code editor and be sure it includes all your code changes, and your name and commit message at the top. maybe a sample? @@ +211,5 @@ > +If everything looks good, you can access the selected bug in [BugZilla](https://bugzilla.mozilla.org) and click on the **Attach File** link located above the first comment. > + > +#### Ask for a Review > + > +When uploading a patch to BugZilla, you can request a review from the user who opened the bug or another developer. Simply select the `?` in the dropdown selector in the _review_ option of the **Flags** section. An input field will appear which will allow you to type the name or username of the user you want to review your patch. \ No newline at end of file Bugzilla is also with lower-case z
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
Here is the patch with your previous review feedback included and the readme.html in the mail directory removed.
Comment 4•4 years ago
|
||
Comment on attachment 9063661 [details] [diff] [review] readme-for-thunderbird-rev1.patch Review of attachment 9063661 [details] [diff] [review]: ----------------------------------------------------------------- ::: README.md @@ -1,2 @@ > # Thunderbird > -Thunderbird an powerful and customizable open source Email client with lots of users. It is based on the Mozilla platform that Firefox is also built on. Why remove that sentence?
Assignee | ||
Comment 5•4 years ago
|
||
Didn't actually mean to remove that sentence, just adjust it. New patch takes care of that Jorg.
Comment 6•4 years ago
|
||
Comment on attachment 9069788 [details] [diff] [review] readme-for-thunderbird-rev2.patch Review of attachment 9069788 [details] [diff] [review]: ----------------------------------------------------------------- Should be ok. But this looks like just an incremental patch. You should include the complete patch for the README.md ::: README.md @@ +25,4 @@ > > ### Build Configuration > > +To build Thunderbird, you need to create a file named `mozconfig` to the root directory of the mozilla-central checkout that contains the option `comm/mail` enabled. You can create a file with this line by doing this in the `source/` directory: there's no "source" directory. It's just the root source directory you mean I think
Assignee | ||
Comment 7•4 years ago
|
||
Magnus, this addresses the changes you requested and both patches have been merged into one.
I think I did all that correctly, so please review at your earliest convenience.
Comment 8•4 years ago
|
||
Comment on attachment 9073969 [details] [diff] [review] readme-for-thunderbird-rev3.patch Review of attachment 9073969 [details] [diff] [review]: ----------------------------------------------------------------- ::: README.md @@ +24,5 @@ > +- [macOS Build Prerequisites](https://developer.thunderbird.net/the-basics/building-thunderbird/macos-build-prerequisites) > + > +### Build Configuration > + > +To build Thunderbird, you need to create a file named `mozconfig` to the root directory of the mozilla-central checkout that contains the option `comm/mail` enabled. You can create a file with this line by doing this in the root source directory: We're talking both about .mozconfig and mozconfig now. Maybe they changed it to allow just mozconfig too due to Windows... but maybe it would be nice to mention that you can use .mozconfig, and then be consistent abotut which file name used @@ +52,5 @@ > +### Building > + > +**Before you start**, make sure that the version you checked out is not busted. For `hg` tip, you should see green Bs on [https://treeherder.mozilla.org/\#/jobs?repo=comm-central](https://treeherder.mozilla.org/#/jobs?repo=comm-central) > + > +To start the build, cd into the `source` directory, and run: plain source, there is no directory named source @@ +98,5 @@ > + > +or to do it via one command: > + > +```text > +hg pull -u; (cd comm; hg pull -u) I think you want hg pull -u && cd comm && hg pull -u Otherwise, on failures you may get into problems (or at least confusions).
Comment 9•4 years ago
|
||
Any intentions to land this?
Comment 10•4 years ago
|
||
Yeah, Ryan said he's working on fixing up the patch.
Assignee | ||
Comment 11•4 years ago
|
||
Here is the newest patch with the changes requested. One commit, as requested.
Comment 12•4 years ago
|
||
Comment on attachment 9085195 [details] [diff] [review] new-readme-for-thunderbird.patch Review of attachment 9085195 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, let's land!
Updated•4 years ago
|
Updated•4 years ago
|
Comment 13•4 years ago
|
||
Let's clarify a few things here first.
This references
https://developer.thunderbird.net/the-basics/building-thunderbird/windows-build-prerequisites
which is an outdated copy of
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Thunderbird_build
:-(
Comment 14•4 years ago
|
||
Some more comments:
- Who's going to keep this up-to-date?
- Why is the "new bug" query https://bugzilla.mozilla.org/buglist.cgi?list_id=14706102&resolution=---&classification=Client%20Software&classification=Developer%20Infrastructure&classification=Components&classification=Server%20Software&classification=Other&query_format=advanced&bug_status=NEW&product=Calendar&product=Chat%20Core&product=MailNews%20Core&product=Thunderbird
useful? It returns 8460 bugs at time of writing. Quite frustrating. - Why are you only mentioning hg bookmarks. Some people use hg queues, so IMHO it should at least be mentioned.
Assignee | ||
Comment 15•4 years ago
•
|
||
As noted in the mailing list thread - MDN is going to remove our stuff. We need to update DTN regularly with the relevant information.
(In reply to Jorg K (GMT+2) from comment #14)
Some more comments:
- Who's going to keep this up-to-date?
You mean DTN? I've tried to get developer's involved to review the content and ensure it is up-to-date.
I will discuss this with Magnus today and see if we can get this task on a staff member's plate to check on these things regularly.
- Why is the "new bug" query https://bugzilla.mozilla.org/buglist.cgi?list_id=14706102&resolution=---&classification=Client%20Software&classification=Developer%20Infrastructure&classification=Components&classification=Server%20Software&classification=Other&query_format=advanced&bug_status=NEW&product=Calendar&product=Chat%20Core&product=MailNews%20Core&product=Thunderbird
useful? It returns 8460 bugs at time of writing. Quite frustrating.
I used what Magnus recommended putting there. But this could be something different. What do you think is most useful?
- Why are you only mentioning hg bookmarks. Some people use hg queues, so IMHO it should at least be mentioned.
I've had some folks point out that hg queues is deprecated and probably not a good thing to encourage.
We do have a full section dedicated to Queues on DTN -> https://developer.thunderbird.net/contributing/fixing-a-bug/using-mercurial-queues
I'm open to feedback. So far I've had a hard time getting any attention to this README. Happy to get feedback.
Comment 17•4 years ago
|
||
"Who's going to keep this up-to-date?" was referring to the readme itself. I guess it doesn't have too many details which will go out-of-date soon, but it's another location with build information which potentially will need updating.
I'd remove/replace the link.
I know that hg queues are supposedly deprecated, but everyone is using them. So would at least mention them.
Comment 18•4 years ago
|
||
Giving users too many options up front is often more cause of confusion rather than being helpful.
Assignee | ||
Comment 19•4 years ago
•
|
||
(In reply to Jorg K (GMT+2) from comment #17)
"Who's going to keep this up-to-date?" was referring to the readme itself. I guess it doesn't have too many details which will go out-of-date soon, but it's another location with build information which potentially will need updating.
I will keep it up-to-date with a recurring calendar reminder to check on it and update it.
I'd remove/replace the link.
What would you replace it with? Happy to do it, just want to know what a good alternative would be.
I know that hg queues are supposedly deprecated, but everyone is using them. So would at least mention them.
The best I can do here is mention "You can also use hg queues to manage your patches to Thunderbird, you can learn more about them here."
Comment 21•4 years ago
|
||
I'd change the paragraph to:
Mercurial is pretty flexible in terms of allowing writing your own code and keeping it separate from the main code base. You can use using Mercurial Bookmarks or Mercurial Queues or for managing your work. We have guides called for bookmarks and queues on our developer website.
I've discussed the issue with John, and personally I don't think bookmarks are up to doing the job. So I'd not recommend anything. Mention bookmarks first if you want, since B comes before Q ;-)
As for the query. I'd but this:
Use the Advanced Search section to find bugs you want to take care of, and be sure that the bug doesn't currently have any user listed as Assignee and the Status is set to NEW
. You can see a list of "easy" bugs for beginners via this query. However, we assume you came here to fix your "pet hate" bug, so you already have a bug to work with.
BTW, the query is: https://mzl.la/2HbDSyX
r+ from me with those changes, haha.
Assignee | ||
Comment 22•4 years ago
|
||
Here is the new patch with the changes requested. Looking forward to a r+! :)
Comment 23•4 years ago
|
||
Comment on attachment 9087157 [details] [diff] [review] new-readme-for-thunderbird.patch Review of attachment 9087157 [details] [diff] [review]: ----------------------------------------------------------------- I can fix the link. Whoever comes first. ::: README.md @@ +170,5 @@ > +Creating an account is necessary in order to submit patches, leave comments, and interact with any other aspect of Bugzilla. If you're currently using an `IRC` username in the `#maildev` channel, we recommend saving your profile name with the current format `Firstname Lastname (:username)` in order to be easily searchable and allow the Thunderbird team to offer better support. > + > +#### Find a Bug > + > +Use the [Advanced Search](https://bugzilla.mozilla.org/query.cgi?format=advanced) section to find bugs you want to take care of, and be sure that the bug doesn't currently have any user listed as _Assignee_ and the _Status_ is set to `NEW`. You can see a list of "easy" bugs for beginners [via this query](https://mzl.la/2HbDSyX). However, we assume you came here to fix your "pet hate" bug, so you already likely have a bug to work with. Sorry, I didn't suggest to use that short query. I'm not sure how permanent it is. Please use the longer one I provided in the same e-mail. r+ with that.
Comment 24•4 years ago
|
||
I've come to fix the link and land this, but it's still not right :/
Here's the bookmark example:
- Run
hg pull
to download the latest changes from the remote repository. - Run
hg update tip
to update your current working directory to the most recent changeset that you just downloaded. - Run the command
hg bookmark some-bookmark-name
to create a bookmark that points to the current changeset. Usually the name of the bookmark is related to the bug or feature you are working on. - Run
hg bookmarks
to list all of your bookmarks and see which one is currently active. The one you just created should now be active. - You can run
hg log
to see which bookmarks point to which changesets. - Make changes to the code until you are ready to commit them.
- Run
hg status
andhg diff
to check what files and changes you are about to commit. If you need to add a new file usehg add some-filename
before committing. - Commit your changes with
hg commit -m "Bug ##### - fixing something amazing"
. The-m
is short for--message
and lets you provide a commit message for your changeset. "Bug ##### - fixing something amazing" is the format we recommend for the commit message, specifying the number of the Bug you're working on and a small description stating what you fixed. Usehg log
to see examples of commit messages. - Run
hg log
and note how the bookmark has moved to the changeset that you just committed. The active bookmark will automatically move to the most recent changeset as you commit your changes. That way you can make a series of commits and the bookmark will always point to the most recent one. - You can use
hg commit --amend
to change your previous commit rather than making a new commit.
You're cheating since you're re-using numbers. Let me give you the much shorter hg queues steps:
0. Run hg pull
to download the latest changes from the remote repository.
- Run
hg update tip
to update your current working directory to the most recent changeset that you just downloaded. - Make changes to the code until you think you covered everything.
- Create a patch for upload with
hg qnew -m "Bug ##### - fixing something amazing"
. The-m
is short for--message
and lets you provide a commit message for your changeset. "Bug ##### - fixing something amazing" is the format we recommend for the commit message, specifying the number of the Bug you're working on and a small description stating what you fixed.
The last command will place a patch into .hg/patches in your repository.
You can now scrap the #### Create a New Patch paragraph completely.
So four steps instead of ten seems much simpler, no? Both instructions are missing the information how to get rid of the local changes. You won't need them since the sheriff commits them to the repository and you will pull them from there.
Updated•4 years ago
|
Comment 25•4 years ago
•
|
||
I have not gotten rid of my habit of using merqurial queue (this was the method of choice back when I started to create patches).
However, it is quite true that the use of HG MQ is highly discouraged now and there are bugs with |mach bootstrap| that interfere with the proper setting of .hgrc so that MQ works correctly.
So at least, we should mention for the benefit of newbies that "support for HG MQ in various mozilla tools is being deprecated, and you are better off to learn how to use bookmarks".
Assignee | ||
Comment 26•4 years ago
|
||
I've thought a lot about this and I am not going to change it to detail mercurial queues as there are a lot of warnings out there in documentation that we shouldn't be using it as it is deprecated. So I have noted that "While some find mercurial queues easier to work with, support for them is being deprecated in various Mozilla tools, so the example below uses bookmarks."
I have also fixed the numbering of the bookmarks instructions.
Let me know what you think, if there is nothing else I would like to see this merged.
Comment 27•4 years ago
|
||
Comment on attachment 9091103 [details] [diff] [review] new-readme-for-thunderbird.patch > Let me know what you think, if there is nothing else I would like to see this merged. I prefer the bookmark section to be removed. We have the link to the documentation, so let's refer people there. There's no need to have the same information in two locations. As per the maildev mailing list thread, the bookmark page needs work, so no point copying an incomplete page into the readme. Also, please check the "space after full stop" style. The typewriter days are over, so we no longer use two spaces after a full stop. Search for ". " (dot space space) in the document. Sorry about anther iteration here.
Assignee | ||
Comment 28•3 years ago
|
||
Hoping this is the final pass. I couldn't find any double spaces that you pointed out at the end of sentences. I did a search in three editors finders and then went line-by-line and checked. Can you point me at where you see one?
I completely removed the hg bookmarks example and just pointed them at documentation. Kept the rest in the READE about what to do after exporting the patch. Let me know what you think.
Comment 29•3 years ago
|
||
Comment on attachment 9093146 [details] [diff] [review] new-readme-for-thunderbird.patch Review of attachment 9093146 [details] [diff] [review]: ----------------------------------------------------------------- OK, to avoid further iterations, I'll fix the issues below and land it. ::: README.md @@ +160,5 @@ > +### [Bugzilla](https://bugzilla.mozilla.org/enter_bug.cgi?product=Thunderbird) > + > +Thunderbird uses bugzilla for reporting and tracking bugs as well as enhancement requests. If you want to become a contributor to Thunderbird, you will need an account on Bugzilla. > + > +## Fixing a Bug and Submitting Patches That was a level ### header, and I don't know why you've changed it to level ## since #### are following. @@ +184,5 @@ > +Once you finished taking care of your favorite bug and using Mercurial to commit and export your patch, you can upload it to Bugzilla for review. > + > +#### Upload a Patch > + > +Open your `patch-name.patch` file in your code editor and be sure it includes all your code changes, and your name and commit message at the top. You can see an example of a patch for this [README here](https://bug1547325.bmoattachments.org/attachment.cgi?id=9061044). I think we can remove the `patch-name.patch` bit here. Maybe that was referenced before in the sections your removed. Also, the ID of the attachment has changed.
Comment 30•3 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/97972953d726
Created README for Thunderbird with build instructions, how to get involved in the community, and a basic guide for contributing. r=mkmelin,jorgk
Updated•3 years ago
|
Description
•