This is a semi-random outpouring of thoughts related to being a Tapestry committer.
IntelliJ. It's a free license for all committers and it's just better. You'll love it after the first couple of days fumbling because you're so used to Eclipse bending you over and doing unspeakable things.
There are shared code formatting settings in support/idea-settings.jar. This will prevent unexpected conflicts due to formatting.
All source files should have a copyright comment on top, except where such a comment would interfere with its behavior. For example, component template files omit the comment.
The year on the copyright should be updated as a file changes. As you are reviewing your changes before checking them in, try to check the coyright date, and add the current year to the list if not present.
Always provide a commit message. I generally try to work off the JIRA, so my commit message is often:
TAPESTRY-1234: Make the Foo Widget more Ajax-tastic!
It is very important to include the JIRA issue id in the commit. This is used in many places: JIRA links issues to the SVN commits for that issue (very handy for seeing what changed as part of a bug fix). Bamboo (the continuous integration server) does something similar.
All Tapestry committers should be registerred with JIRA and part of the tapestry-developers JIRA group.
I work the following JIRA list: JIRA Work Queue.
Ideally, we would always work top priortity to low priority. I (Howard) sometimes jump out of order, if there's something cool to work on that fits in an available time slot. Alternately, you are always allowed to change the priority of a bug before or as you work it.
When you start to work on an issue, make sure it is assigned to you and use the start progress option.
I often add comments about the state of the fix, or the challenges. This often spurs the Issue's adder to provide more details.
I often update the issue description to make it more legible and more precise, i.e., "NPE in CheckUpdates" might become "NullPointerException when checking for updates to file that have been deleted". Verbose is good.
Is it a bug fix without tests? No. In some cases, I write new tests to prove that an issue is not valid and then leave the tests in place -- the close the bug as invalid.
A good plan is to write a test that fails then work the code until the test passes. I'm also surprised by how often code works in a unit test but fails unexpectedly in an integration test. As the G-Man says, "Expect unforseen consequences".
When you check in a fix, you should close the issue and make sure the fix release is correct.
We're playing fast and loose -- a better procedure would be to mark the bug resolved and verify the fix before closing it. That's ok, we have a community to double check our work :-).
For anything non-trivial, wait for the Bamboo server to build. It catches a lot of things ... such as files that were not added to SVN. And even IntelliJ has a bit of trouble with wildly refactored code. Bamboo will catch all that.
Always provide comments about why an issue is invalid ("A Ruby implementation of Tapestry is out of scope for the project."), or at least, a link to the duplicate issues.
Close the issue but make sure the fix release is blank. Otherwise, the issue will be listed in the release notes, which we don't want.
The ASF copyright must appear on all Java source files. Technically it should appear on all non-binary files in the repository.
As you make changes to files, update the copyright to add the current year to the list. The goal is that the copyright notice includes the year in which files change. When creating a new file, don't back date the copyright year ... start with the current year. Try not to change the copyright year on files that haven't actually changed.
IntelliJ has a great comparison view: Cmd-9 to see the local changes, the Cmd-D to see the differences. I whip through the changes (using Cmd-forward arrow) and make sure copyrights are up to date as I review the changes prior to a commit.
This is a real big deal. As long as code is in the internal package, we have a high degree of carte-blanche to change it. As soon as code is public, we become handcuffed to backwards compatibility.
Interfaces are public, implementations are private. You can see this is the bulk of the code, where org.apache.tapestry.services is almost all interfaces and the implementations are in org.apache.tapestry.internal.services.
Many more services have both the interface and the implementation in org.apache.tapestry.internal.services.
We absolutely do not want to make Page or ComponentPageElement public. You will often see public service facades that take a page name as a method parameter, and convert it to a page instance before invoking methods on internal services.
We do not have a specific plan for this yet. Future Tapestry 5 will add features to allow clean renames of parameters, and a way to deprecated and eventually remove components.
Yes, I (Howard) use leading underscores for field names. I just prefer that to prefixing with this., you don't have to do so, but try to make your code blend in when modifying existing source.
Long ago, Tapestry code use the regrettable "leading-I-on-interfaces" style. Don't do that. Everythings an interface.
I prefer braces on a new line (and thus, open braces lined up with close braces), so that's what the default code formatting is set up for. I sometimes omit braces for trivial if statements, such as return;. I use a lot of vertical whitespace to break methods into logical sections.
We're coding Java, not Pascal; I'd much rather see a few checks early on with quick returns or exceptions than have ten-levels deep block nesting just so a method can have a single return statement. In other words, else considered harmful. Low code complexity is better, more readable, more maintainable code.
I don't bother alphabetizing things, because I have an IDE that lets me jump around easily.
Final is the new private. Final fields are great for multi-threaded code. Especially when creating service implementations with dependencies, store that dependencies into final fields. Once we're all running on 100 core workstations, you'll thank me. Seriously, Java's memory model is seriously twisted stuff, and assigning to a non-final field from a constructor opens up a tiny window of non-thread safety.
Comments are overwhelmingly important. Try to capture the why of a class or method. Add lots of links, to code that will be invoked by the method, to related methods or classes, and so forth.
Comment the interfaces and don't get worked up on the implementations. Javadoc does a perfectly good job of copying interface comments to implementations, so this falls under the Dont Repeat Yourself guideline.
Be very careful about documenting what methods can accept null, and what methods may return null.
Naming things is hard. Names that make sense to one person won't to another.
That being said, I've tried to be somewhat consistent with naming. Not perfectly.
Objects that are exposed to user code should generally implement a meaningful toString() method. And that method should be tested.