WordPress.org

Ready to get started?Download WordPress

Make WordPress Core

Updates from Andrew Nacin Toggle Comment Threads | Keyboard Shortcuts

  • Andrew Nacin 6:49 pm on January 15, 2014 Permalink | Log in to leave a Comment
    Tags:   

    Git mirrors for WordPress 

    I’m pleased to share we now have an official Git mirror for the WordPress core development SVN repository.

    git clone git://develop.git.wordpress.org/

    Read-only mirrors are also set up for a few other WordPress.org repositories, including BuddyPress, bbPress, and the old core.svn “build” repository:

    git clone git://buddypress.git.wordpress.org/
    git clone git://bbpress.git.wordpress.org/
    git clone git://core.git.wordpress.org/
    git clone git://glotpress.git.wordpress.org/

    Make sure you use the git protocol (not http) and include the trailing slash.

    Can you use develop.git.wordpress.org for core development? Yes! For all practical purposes, the SVN and Git repositories are now equals. Pick your poison; use whatever you’d like for all your development and deployment needs.

    For creating patches: If you’re into the command line, simply use git diff — to be specific, git diff --no-prefix is ideal. This format is easily applied with patch, just like SVN diffs. For more, check out scribu’s post on contributing using git. Might also be a good time to plug grunt-patch-wordpress, a work in progress — help make it awesome, if you can.

    With this mirror, we’ve applied some lessons we learned from previous experiences:

    • Tags for major releases receive an extra .0 (“3.8.0″, not “3.8″), making them compatible with tools requiring semver-like version numbers, and allowing branches to have their own namespace (“3.8″, not “3.8-branch”).
    • Committer data is pulled from WordPress.org.
    • The SSL versions of the SVN repositories are used.

    Note that these Git repositories have different hashes than anything currently mirrored on GitHub. We never did break these hashes as promised a year ago. Next steps will be to figure out how to best mirror these to GitHub (and replace what’s there), which is complicated by now having old and new repositories for core development. Additionally, I’m sure we’ll find at least one glitch in this in a matter of a few days, so consider the next week or so a beta test.

    Edit: Well, I mentioned the next week would be a beta test. We found an error (that didn’t take long) in how authors got synced over, so we’ll be breaking the hashes tonight.

    There are a lot of other repositories on WordPress.org not yet mirrored. Notably: plugins and themes. These are massive multi-project repositories and will require a bit more investment.

     
    • Amy Hendrix (sabreuse) 6:51 pm on January 15, 2014 Permalink | Log in to Reply

      w00t!!

    • Boone Gorges 6:52 pm on January 15, 2014 Permalink | Log in to Reply

      OMG. git-svn, I will not miss you. Thanks @nacin!

      • Andrew Nacin 6:56 pm on January 15, 2014 Permalink | Log in to Reply

        To clarify, as I think I chopped out an sentence when editing this: It’s still a read-only mirror. “For all practical purposes” was meant to apply to everyone but committers. So for the committers to these repos that want to use Git, you’ll still need to use git-svn, for the moment.

    • Travis Northcutt 6:52 pm on January 15, 2014 Permalink | Log in to Reply

      Awesome!

    • Ionel Roiban 6:53 pm on January 15, 2014 Permalink | Log in to Reply

      Great! How about an official repo on Github?

      • Andrew Nacin 6:58 pm on January 15, 2014 Permalink | Log in to Reply

        We’ve had an official mirror there for a long while now. But this is subject to some changes; see the second-to-last paragraph in this post.

        • Ionel Roiban 7:20 pm on January 15, 2014 Permalink | Log in to Reply

          Thank you. I did actually have that Github repo watched but its description makes it clear that it is just a mirror (from svn) and not a place where anyone can submit their contributions to. What’s the plan on accepting pull request directly on Github, if any?

          • Andrew Nacin 7:25 pm on January 15, 2014 Permalink | Log in to Reply

            I’ve been studying how some other projects do or don’t do this. Something to discuss in a few months, I imagine.

            • Weston Ruter 11:12 pm on January 15, 2014 Permalink

              @nacin yeah, it is amazing to see that Symfony2 recently (and rightfully) boasted about having over 10,000 pull requests on their GitHub repo: https://github.com/symfony/symfony/pulls

              I can’t imagine how many Core pull requests would come in if WordPress accepted them on GitHub.

            • Mike Schinkel 8:18 pm on January 19, 2014 Permalink

              @Weston – LOL. Too many pull requests might be a “be careful what you wish for” concern than that the core team might want to avoid! :)

    • deltafactory 6:54 pm on January 15, 2014 Permalink | Log in to Reply

      Please excuse me if this had been answered previously:

      What’s the roadmap, if any, for deprecating SVN for core development and/or plugins?

      • Andrew Nacin 7:04 pm on January 15, 2014 Permalink | Log in to Reply

        If you or your tools want to use Git, you should be able to. If you or your tools want to use SVN, you should be able to. You could think of it as a desire to be VCS agnostic. Whether one commits to SVN or pushes to Git is ideally a decision that should rest with individual plugin developers. Or for the case of these repos, the group of committers. When the infrastructure is there for this, of course.

    • Thomas van der Beek 6:57 pm on January 15, 2014 Permalink | Log in to Reply

      Yippie-kai-yay! Can’t wait for plugins and themes repo’s!

    • Drew Jaynes 6:58 pm on January 15, 2014 Permalink | Log in to Reply

      This is great news! If/when plugins and themes are mirrored over, it will be a huge boon as well. Thanks for all your work on rolling this out :)

    • Bryan Petty 7:12 pm on January 15, 2014 Permalink | Log in to Reply

      For anyone that wasn’t currently aware, there has been a large on-going effort to do the same for plugins for the last year already, currently mirroring up to GitHub: Plugin Mirror

      It’s still more of an experiment and utility than an officially supported workflow though.

    • Cor van Noorloos 7:20 pm on January 15, 2014 Permalink | Log in to Reply

      Probably in too much of a hurry, but will http://svn.automattic.com/wordpress-i18n/pot/trunk/ also receive its git mirror?

    • Andrew Nacin 7:24 pm on January 15, 2014 Permalink | Log in to Reply

      The post makes clear that these operate over git://. The reason is this is simply being served using git-daemon. We’ll work on support for http-based communications in the future. If your organization blocks git and requires http or https, you can easily mirror this yourself for the moment (and stay tuned for these to be pushed to GitHub).

    • Julien 7:30 pm on January 15, 2014 Permalink | Log in to Reply

      Aaah so awesome! Thanks for making this happen !

    • Mike Schroder 7:44 pm on January 15, 2014 Permalink | Log in to Reply

      Woohoo! I’d been waiting for this. Thanks muchly!

    • Andrew Nacin 7:49 pm on January 15, 2014 Permalink | Log in to Reply

      So, I did mention the next week would be a beta test. We found an error (that didn’t take long) in how authors got synced over, so we’ll be breaking the hashes tonight.

      • Daniel Bachhuber 10:19 pm on January 15, 2014 Permalink | Log in to Reply

        Just to clarify: this means github.com/WordPress/WordPress will be broken (or worse, incorrect) for everyone that’s added it as a submodule in their projects? Or am I missing something?

        • Andrew Nacin 10:34 pm on January 15, 2014 Permalink | Log in to Reply

          Sorry, I meant the hashes of these git.wp.org repositories. We haven’t figured out what the plan will be for the GitHub repository yet. We already signed off on breaking those hashes in the past, but I don’t know how feasible that is. Options include moving wordpress/wordpress to wordpress/wordpress-build (and then optionally breaking the hashes and having it mirror core.git) and then adding a new wordpress/wordpress-develop for develop.git (one downside being the watchers and stars wouldn’t carry to the new “canonical” repo, but not a big deal). Suggestions welcome.

        • Daniel Bachhuber 10:42 pm on January 15, 2014 Permalink | Log in to Reply

          Phew :) Glad I don’t have to work late tonight. I don’t have any good suggestions at the moment.

    • WebSharks 8:08 pm on January 15, 2014 Permalink | Log in to Reply

      This is awesome! Hooray :-) Can’t wait to see this extended to plugins/themes.

    • Gregory Cornelius 8:12 pm on January 15, 2014 Permalink | Log in to Reply

      You made my day.

    • Stephen Edgar 9:04 pm on January 15, 2014 Permalink | Log in to Reply

      Very Cool, Thanks. :)

    • Pippin Williamson 9:04 pm on January 15, 2014 Permalink | Log in to Reply

      Fantastic!

    • Rafael Funchal 10:33 pm on January 15, 2014 Permalink | Log in to Reply

      This is really awesome.

    • Nashwan Doaqan 10:21 am on January 16, 2014 Permalink | Log in to Reply

      Coool!

    • Frank 4:36 pm on January 16, 2014 Permalink | Log in to Reply

      Great to see, that WordPress opens for git.

    • Chuck Reynolds 5:20 am on January 17, 2014 Permalink | Log in to Reply

      big step.. love it.

    • Eduardo Reveles 11:48 pm on January 17, 2014 Permalink | Log in to Reply

      Nice!

    • avoryfaucette 10:35 am on January 18, 2014 Permalink | Log in to Reply

      Omg so exciting! I’m really stoked to hear that WordPress is moving in the direction of fully supporting Git and maybe eventually GitHub.

      I have a question that I hope isn’t too newbie for here. I understand the issues above with GitHub and I can handle using the command line. However, I’m new to open source contributions and would like to start with working on small WordPress bugs. I have a GitHub profile and have installed GitHub’s software to my Mac to keep track of my forks locally. So I have two questions:

      1) For my local purposes, does this mean I could theoretically keep track of my own work in the GitHub software, even though I can’t actually use GibHub to submit patches? If yes, would I just follow scribu’s instructions through step 3 (make a feature branch) and then locally save the clone of the WordPress source files so my GitHub software can see it?

      2) For “having things on my GitHub profile purposes,” once I’ve followed to command line steps to submit my patch, would I just use the usual process to send the patch to GitHub but not actually create a pull request?

      Thanks for helping out a new contributor!

      • Andrew Nacin 11:02 pm on January 18, 2014 Permalink | Log in to Reply

        Right, you can still create a feature branch, commit to that branch, and push that branch to your own fork on GitHub. You just would not actually create and send a pull request. Note that you can also get a patch from GitHub by adding “.diff” to the end of any commit URL.

    • Todd Lahman 1:08 am on January 25, 2014 Permalink | Log in to Reply

      Looking forward to the day when the plugin repo is gitified.

    • Denis de Bernardy 6:22 am on January 31, 2014 Permalink | Log in to Reply

      git clone git://buddypress.git.wordpress.org buddypress
      Cloning into ‘buddypress’…
      fatal: unable to connect to :
      [0: ::1]: errno=Connection refused
      [1: 127.0.0.1]: errno=Connection refused

      • Denis de Bernardy 6:25 am on January 31, 2014 Permalink | Log in to Reply

        Also, when I clone the same using http, I get:

        $ head readme.md

        1. WordPress for Android #

        i.e. not the correct repo.

      • Andrew Nacin 6:26 am on January 31, 2014 Permalink | Log in to Reply

        You need a trailing slash in addition to the git protocol.

        • Denis de Bernardy 9:14 am on January 31, 2014 Permalink | Log in to Reply

          OK, that works. A redirect or a more meaningful error message would be welcome… As would making the http version of the repo serve the correct repo. :-)

          • Andrew Nacin 9:17 am on January 31, 2014 Permalink | Log in to Reply

            I’m not sure why the slash is needed, but based on the error I presumed it was at the git client level, not something on our end. I honestly have no idea.

            The git protocol is served by the git daemon. The http protocol is served by gitolite, which was never actually deployed; I’ll get that turned off for now to alleviate confusion.

    • helgatheviking 3:13 pm on April 1, 2014 Permalink | Log in to Reply

      I too am seeing the fatal error. I was able to clone ok, but now when I need to pull:

      fatal: unable to connect to develop.git.wordpress.org:
      develop.git.wordpress.org[0: 66.155.40.244]: errno=No error

  • Andrew Nacin 5:31 pm on January 10, 2014 Permalink | Log in to leave a Comment
    Tags: reporting,   

    Brainstorming ticket reports 

    One thing I’d like to work on is improve our reports on Trac with the hope we can better manage the ticket queues. We currently have about 50 reports on Trac. Many of them were created in a push 5-6 years ago to build reports around keywords. A number of others are one-offs created in the last few years for specific use cases. In reality, few of these are ever used on a day-to-day basis.

    I checked the access logs to get an idea of the most popular reports. Besides the first six reports (my favorite “home base” is report 6), they are report 16 (“Needs Patch”), report 18 (“Blockers for Releases”), and report 21 (“Latest Tickets”).

    What I’d like to do is brainstorm all sorts of new reports we should have. How can we make what we have better? I’ll then go to work about implementing them. Dream big — if the data’s there, I’ll figure out the gnarly SQL required. We can also think about how we can improve columns, grouping, ordering, etc. If you have a concept for a report but aren’t sure how we’d query for those tickets, toss the idea out there and maybe someone will add to it.

    I’l probably design a new /report screen so it’s not just a list ordered by something random like when the report was created, and so we can highlight important reports, group similar reports together, and put less emphasis on some of the more specialized ones.

    Here are some half-baked ideas, to start us off:

    • Open tickets without a response. Find all tickets where no one has commented, or where only the reporter has commented. This would create a punchlist of tickets to review. (Ideally, the report should always be empty.) The ability to group or filter this by component would be helpful, for contributors who specialize in and want to take responsibility for different areas.
    • Report of “dead” tickets. Tickets that haven’t been modified in four years are ripe for action and/or closure. We should empty a report of this nature, then reduce it to 3.5 years, then 3 years, etc.
    • Good first bugs. A report of all tickets marked as a good first bug. (None yet, as the keyword is new.) Next steps: How can we best identify these kinds of tickets?
    • Report of “untriaged” tickets. Our milestones have gotten out of hand, which is something we should discuss. Tickets should immediately leave “Awaiting Review” once they’ve been initially reviewed, but then not forgotten about if they are shifted to “Future Release”. Let’s figure out metrics for what makes a ticket “untriaged”. We could create a simple UI to filter by component for a report like this. We currently have Tickets Awaiting Review which groups tickets by the “Version” they were reported against — maybe this is a good start.

    What’s your idea?

     
    • Helen Hou-Sandi 5:35 pm on January 10, 2014 Permalink | Log in to Reply

      Candidates for closure. Not 100% sure what this would look like beyond the “close” keyword (perhaps semi-related to “dead” tickets?), but could be a good report, especially for those getting into Trac gardening. Dreaming big!

      • Andrew Nacin 5:39 pm on January 10, 2014 Permalink | Log in to Reply

        I’m not sure what this would look like either, hence this exercise. :-) Closing out tickets that shouldn’t be open any longer is always a fine idea! The keywords “close”, “2nd-opinion”, “reporter-feedback” are all good for this. Some kind of combination of Needs Reporter Feedback / Steps To Reproduce and Suggesting Close / Needs 2nd Opinion (that’s close to 200 tickets right there).

        Not sure what else — yeah, I’d say it’s related to “dead” tickets. Once “Awaiting Review” is under control again, it could be used to identify the oldest tickets still awaiting review.

    • Andrew Nacin 5:35 pm on January 10, 2014 Permalink | Log in to Reply

      An “intelligent” report on a component. For a particular component, group and order tickets in a way that helps keep the component well-maintained. The first group would be “slated for the next release,” for example. Other groups could include “Tickets awaiting review”, “Tickets more than X years old”, “Tickets needing feedback,” “Tickets with patches,” and (if they don’t fit into another group) “Major bugs,” “Minor bugs,” and “Enhancements.” The report should probably also try to render the component’s ticket graph at the top so we can see changes over time.

      A report like this could be a prime home base for a contributor wanting to clean up and be responsible for a component. What other groups would make sense?

      • Jeremy Felt 6:04 pm on January 10, 2014 Permalink | Log in to Reply

        Somewhat out of scope… is there a way to relate a ticket to other components. When a major effort is taken in Administration, then a report of ‘Multisite tickets that could be affected’ would be interesting.

    • Morgan Estes 5:56 pm on January 10, 2014 Permalink | Log in to Reply

      Not sure what to call it, but something for patches submitted for a release but not responded to by the time of the release freeze. I have a couple like this that I’m unsure of the status (realizing that just because it’s submitted doesn’t mean it’s used), and are in Awaiting Review.

    • Jeremy Felt 5:58 pm on January 10, 2014 Permalink | Log in to Reply

      Along the same lines of dead tickets, I wonder if there’s use in a view of all tickets reported against a version X releases old. These should be ripe for verification as they may have been resolved unknowingly along the way.

    • Andrew Nacin 6:00 pm on January 10, 2014 Permalink | Log in to Reply

      A report of tickets that have a patch uploaded to them no one has replied to yet. We’d do this by finding the most recent patch upload for a ticket, then seeing if there were any comments (by someone other than the uploader) that had been posted after.

    • Jeremy Felt 6:02 pm on January 10, 2014 Permalink | Log in to Reply

      Last modified might take care of this, but “Open tickets without a response in 30 days” or “Open tickets without a response in 2 releases”

      • Andrew Nacin 6:08 pm on January 10, 2014 Permalink | Log in to Reply

        The report of “dead” tickets mostly covers this, I think. But taking it further: perhaps “without a response” are the operative words here. Response from whom? How about open tickets where the last person to reply is the reporter? That tells me they are waiting for a reply from someone else.

        • Jeremy Felt 6:10 pm on January 10, 2014 Permalink | Log in to Reply

          Not sure if we can get fancy with roles, but “without response from a lead developer” or “…from active component person” or “…from bug gardener” or “…from committer” could all be interesting.

          • Andrew Nacin 6:15 pm on January 10, 2014 Permalink | Log in to Reply

            I was just thinking about that. Tickets without a response from any of X individuals is interesting. Of course, Sergey and I mostly cover the spread on that. :-) Maybe tickets where the last person to comment is not one of those people.

            We could probably come up with a way to do this on a per-component basis (so, all multisite tickets Jeremy has never looked at).

    • Andrew Nacin 6:05 pm on January 10, 2014 Permalink | Log in to Reply

      I think there are some good opportunities to highlight “extremes”. For example: Which open tickets have the most comments?

      The most votes isn’t that useful of a metric yet (and also, because it’s mostly just long-standing tickets like post relationships), but what about comparing/contrasting votes and comments? It’s really interesting to see a really long ticket that has almost no interest. #22692 has 67 comments yet just one vote, which tells me it’s a bit of an edge case and a time suck. Compare that with 68 comments but 31 votes on #19393.

      High number of attachments is interesting. So could a ratio of comments to attachments? Dunno. Lots of possibilities here.

    • Drew Jaynes 6:28 pm on January 10, 2014 Permalink | Log in to Reply

      From a docs perspective, it would be lovely to have a report that gathers needs-docs, needs-codex, and docs-feedback together. Right now we have:

      • Report 12 (needs-unit-tests/needs-docs)
      • Report 36 (needs-codex)
      • Report 37 (needs-unit-tests)

      Seems like that would also free up the needs-unit-tests duplication between reports 12 and 37.

      • Andrew Nacin 6:52 pm on January 10, 2014 Permalink | Log in to Reply

        Should needs-docs, needs-codex, and docs-feedback all be different groups on this report? What would they be called? “Needs Inline Documentation”, “Needs Codex Documentation”, “Needs Feedback on Documentation”?

        • Drew Jaynes 7:04 pm on January 10, 2014 Permalink | Log in to Reply

          I’d probably group them separately, yes. And ouch on the verbosity, but I guess that’s probably necessary — we can’t expect everyone to understand Docs == Documentation ;)

          It just occurred to me that there also might be room for including tickets on the Text Changes component. Though text changes kind of serve a different master, it’s all documentation in one form or another in the end.

    • Nick Halsey 6:42 pm on January 10, 2014 Permalink | Log in to Reply

      For Twenty Thirteen and Fourteen development we used a verbose custom query along the lines of https://core.trac.wordpress.org/query?status=accepted&status=assigned&status=new&status=reopened&status=reviewing&component=Bundled+Theme&summary=~Fourteen&group=milestone&col=id&col=summary&col=status&col=owner&col=type&col=priority&col=milestone&order=priority (try searching the #wordpress-themes logs for it, it changed a few times too).

      It would be cool if we could create a report that achieves the same effect, allowing us to select a particular bundled theme’s open tickets.

      • Andrew Nacin 6:50 pm on January 10, 2014 Permalink | Log in to Reply

        I wish I had heard about this. We could have made it a saved report with just a few clicks. :-)

      • Robert Dall 6:53 pm on January 10, 2014 Permalink | Log in to Reply

        Nick beat me to the punch on this suggestion. But a total +1 on that.

        We could name it:

        “Current Default Theme Development”

        Or something like that…

    • Eric Andrew Lewis 6:49 pm on January 10, 2014 Permalink | Log in to Reply

      Some reports are also not 100% accurate, because tickets require a human-touch to change keywords. Sorry to go a bit tangential to this topic, but I’d like to suggest we improve report integrity by adding some automation here. There are a few ways we could approach this, I’ll just give one that’s been on my mind for a while, and I’m sure others’.

      Automated patch cleanliness testing, which could automatically toggle ‘has-patch’ // ‘needs-refresh’ keywords via a bot.

      • Andrew Nacin 6:51 pm on January 10, 2014 Permalink | Log in to Reply

        This is doable! We could have a bot go through and apply the latest patch on a ticket. If it doesn’t apply, we could add needs-refresh. Any other ideas along these lines?

        • Jeremy Felt 6:55 pm on January 10, 2014 Permalink | Log in to Reply

          We could have a bot go through and apply the latest patch on a ticket. If it doesn’t apply, we could add needs-refresh.

          +1withmany0s

        • Andrew Nacin 6:56 pm on January 10, 2014 Permalink | Log in to Reply

          If we did this, I feel we’d have this bot do a few other things too. So, apply the patch and run unit tests, for example. (This would need to be sandboxed. Even applying and reverting patches would need to be done carefully.)

          • Helen Hou-Sandi 6:59 pm on January 10, 2014 Permalink | Log in to Reply

            Was going to suggest running unit tests post-patch. +1 to that.

            • Andrew Nacin 7:02 pm on January 10, 2014 Permalink

              I imagine it would not be too difficult to integrate Travis CI into all of this. @bpetty?

            • Bryan Petty 7:15 pm on January 10, 2014 Permalink

              We could potentially trigger patch builds by scripting up automated pull requests. It’d be a bit of work, but I’m pretty sure this would be easier to do from Jenkins CI. @kurtpayne still has a Jenkins CI setup running, it’s a little out of date (I don’t think he’s actually updated it to use the develop repo still), but he probably has a better idea of how we could automate it.

              http://jenkins.kpayne.co:8080/

            • Bryan Petty 7:18 pm on January 10, 2014 Permalink

              Another option might be to setup a copy of Gerrit working off a git mirror.

            • Bryan Petty 7:29 pm on January 10, 2014 Permalink

              For what it’s worth, MediaWiki uses a combination of both Jenkins and Gerrit, and uses Jenkins bot to make several different checks to automatically sign-off patches in Gerrit. Maybe in our case, just Jenkins + Trac is good.

              Good checks to make can include:

              • PHP lint check
              • CodeSniffer with WP rules (this can be iffy though where existing code base isn’t already entirely compliant).
              • Unit tests
        • Morgan Estes 6:57 pm on January 10, 2014 Permalink | Log in to Reply

          That’s excellent! I’ve wondered how needs-refresh gets applied, and this would be a good addition to the “has anyone seen this yet?” status.

          How would it handle multiple patches in a ticket that deals with multiple fixes for a solution?

          • Andrew Nacin 7:02 pm on January 10, 2014 Permalink | Log in to Reply

            It wouldn’t be able to handle multiple patches, because it wouldn’t know the difference between an old patch (that is stale and has been updated) versus an alternate patch. I think that’s OK, this is definitely better than not having it at all.

        • Helen Hou-Sandi 6:57 pm on January 10, 2014 Permalink | Log in to Reply

          Automatically adding has-patch and triggering the notification for that would be good, since uploading attachments doesn’t. Would probably have to be a little smart about what’s a patch vs. not, but could be cool. False positives would be better than missed patches, in any case.

          • Andrew Nacin 7:22 pm on January 10, 2014 Permalink | Log in to Reply

            It could look around for *.patch or *.diff uploaded and have been there for at least an hour. (If the person is going to comment, we can assume it’d be within an hour.) Then we can check if the report has has-patch. If it doesn’t, we can have a bot post a comment and add has-patch.

            Even if the person has commented, it’s possible they didn’t add has-patch. We could still add the comment in that case. That said, it’s possible their comment is to say “Ignore my patch, that won’t work.” How do you differentiate between a user who didn’t add has-patch accidentally, and a user who didn’t add it on purpose?

            So, new approach. Two different angles. One, a bot that goes around and adds has-patch if the user hasn’t commented with an hour. Two, some JS that looks to see if a patch was recently uploaded by the current user. If it has, it Clippy-prompts the user, letting them know to post a comment and add has-patch.

            Or, we could provide feedback after uploading a patch that tells you to post a comment and add has-patch to the ticket.

            “Hey, you’ve uploaded a patch! Be sure to *add a comment to the ticket*.” And that link will not only take them to the new comment form, but it’ll automatically add the has-patch keyword for them.

        • Mike Schroder 12:52 am on January 11, 2014 Permalink | Log in to Reply

          I really like this idea.

    • Andrew Nacin 6:59 pm on January 10, 2014 Permalink | Log in to Reply

      Needs unit tests. Some contributors may just like writing unit tests. We used to say that a ticket would receive needs-unit-tests until they got committed. But now that tests and src are in the same repository, tests in most cases should be committed with the code they are testing. This is good, but means we need to tweak things a bit.

      So what we could do is this:

      • needs-unit-tests is added to an open ticket. This adds it to a report.
      • Someone writes unit tests. The keyword is removed, and it falls off the report. At some point, the tests are committed with the patch on the ticket, and things are good to go.

      And for closed tickets:

      • A ticket is closed, but it could still benefit from unit tests, so the needs-unit-tests keyword is added. This adds it to the same report, under a separate section probably.
      • Someone writes the unit tests. The keyword is replaced with “has-unit-tests”, and it is moved to another section on the same report saying “Tests Ready for Commit” (which would only list closed tickets with has-unit-tests).
      • Once committed, the has-unit-tests keyword is then removed.
    • John Blackbourn 7:44 pm on January 10, 2014 Permalink | Log in to Reply

      Plugin candidates. Open feature requests which have some comments or CCs but no patch or only stale patches. Candidates for someone to build the feature request as a plugin. Might also give us a chance to close the unwanted ones.

    • John Blackbourn 7:51 pm on January 10, 2014 Permalink | Log in to Reply

      Needs dealing with. Open tickets reported against versions more than two versions old that are marked as high or highest priority, or major/critical/blocker severity. Basically these need dealing with.

    • Aaron Jorbin 9:27 pm on January 10, 2014 Permalink | Log in to Reply

      Kind of off the wall, but I would love a report that scanned all patches to see if they add new JavaScript unit tests so that it was easy to review all potential JS Unit tests.

    • Kurt Payne 2:36 am on January 11, 2014 Permalink | Log in to Reply

      Responding to @Bryan Petty … I haven’t kept jenkins up to date. I do, however, have some scripts that may prove usefulf for this. The idea is this:

      • Create a clean development chroot
      • Check out wordpress, create a database, etc.
      • Apply the patch to the source
      • Run the unit tests
      • Display the output
      • Destroy the chroot

      It still takes a few mins to do, but it’s a very good way to get basic feedback on patches like “does it apply cleanly? does it break unit tests?” Let me know if you want to see. It has not been updated with the latest (3.7) svn layout.

      • Bryan Petty 6:25 am on January 11, 2014 Permalink | Log in to Reply

        As far as that goes, it’s way easier than it used to be with two checkouts, and I did get that all written for our Travis CI config. I just figured you’re much more familiar with Jenkins than I am, and might know how to best configure the build configs, and how to trigger Jenkins on Trac patch uploads. :)

        But I can certainly look into it myself too when I have the time.

    • Kurt Payne 7:16 pm on January 11, 2014 Permalink | Log in to Reply

      @bpetty I can help with Jenkins, sure.

      Travis is probably the best way to go for just unit tests.

      Jenkins is going to be more flexible for things like static analysis tools, customizing reports, integrating with other tools (e.g. Sonar is a popular dashboard), triggering downstream builds, potentially scaling by adding build slaves and potentially adding selenium tests (if wordpress ever has them).

      With regards to the “test the patches” script above … we can talk more offline about it … the important question is: Do we want something that automatically tests patches?

    • Sam Sidler 8:17 pm on January 14, 2014 Permalink | Log in to Reply

      A bit late to the party, but I’ve read through all the comments here and it seems like we have a bunch of good reports already that are hard to find. I think we should redo the Reports page to group together specific reports and make it easier to do specific component queries.

      The specific reports in each section might not be quite right in this mockup, but basically this is what I’m thinking: http://i.imgur.com/R7uNrJo.png

      (Even merged needs-docs, needs-codex, and docs-feedback together for a “Needs Docs Team” report.)

      For certain reports that are a combination of reports (like the Needs Docs Team report), we could easily list out what it includes underneath along with links to those specific reports.

      I’d also want to consider adding a “Open tickets by keyword” option on the right side, similar to the component one.

      All of this should be pretty easy to implement, it’s just a matter of deciding how much we want to round the corners of the boxes. ;)

  • Andrew Nacin 10:19 pm on January 9, 2014 Permalink | Log in to leave a Comment
    Tags: notifications,   

    New Trac notifications feature 

    Monday’s long list of improvements to Trac was just the first course. Introducing per-ticket notifications.

    Just above the comment box, you’ll see a new notifications pane. Here, you can watch/unwatch tickets.

    This means Trac’s terrible email address CC feature is gone. (Last night, I migrated over about 15,000 CCs going back nine years.) Old comments that are just CCs are now hidden via JS, which makes a few of the busier or popular tickets easier to skim.

    You can also click the star next to the ticket number at the top:

    Screen Shot 2014-01-09 at 4.38.22 PM

    Watching a ticket adds it to some new reports: open tickets I’m watching and all tickets I’m watching. Now your list of starred tickets are all in one place.

    Coming soon: You’ll be able to watch/unwatch a ticket directly from a report, like starring conversations in a Gmail inbox (edit: added). You’ll also be able to subscribe to entire components and milestones (here’s a sneak peek). And, @-mentions will let you add someone to a ticket.

    A note on how this is implemented: isn’t just subscribe/unsubscribe. “Watch” can also be thought of as “Vote” or “Star” or “Favorite”. We’ll soon add a star count to reports (next to the new Comments column — edit: added). If you’re watching a ticket, you’ll always get notifications. If you’re not, you might still get notifications if you reported the ticket, or are subscribed to that ticket’s component or milestone, or if you’ve been mentioned (and the notifications box will tell you that). From there, you have the option to actively “Watch” the ticket, passively continue to receive notifications, or specifically mute/block notifications coming from that ticket.

    Follow-up question from @sabreuse, about watching/voting/starring/favoriting: If you subscribe to the “firehose” (wp-trac mailing list) and receive all notifications anyway, should you star tickets anyway as an indication of interest? Yes, I’d say so! (If you receive the mailing list to the same email address as your WP.org profile, you’ll only receive one email. It’s possible your mail client could ascertain you received it as a BCC versus just via a mailing list, though I’m not sure.)

    Feedback, ideas, suggestions are very much appreciated. Also, all of this code is open source (a lot of it is a WordPress plugin).

    Edit, bonus: @ocean90 converted most icons on Trac into Dashicons.

     
    • Janneke Van Dorpe 10:26 pm on January 9, 2014 Permalink | Log in to Reply

      Awesome! What about notifications that are set on profiles.wordpress.org? :D

      • Andrew Nacin 10:32 pm on January 9, 2014 Permalink | Log in to Reply

        For @-mentions and such? Yeah, I’m going to try to pipe mentions through that. That said, what should the emails look like? Should they be Trac content piped through the HTML emails, or should you receive a Trac email? I think it’ll end up being Trac-style emails for @-mentions but the HTML emails for other string matches… Still trying to figure out how I’m going to tackle it.

        • Janneke Van Dorpe 10:44 pm on January 9, 2014 Permalink | Log in to Reply

          I’d be really nice if they’re all HTML emails, converting Trac’s markdown. But some people probably prefer plain text.

          • Andrew Nacin 10:49 pm on January 9, 2014 Permalink | Log in to Reply

            I just meant they are two completely different templates – mentions emails and Trac emails. Which one to use?

            • Janneke Van Dorpe 9:34 am on January 10, 2014 Permalink

              I don’t know, but I’d use the same one for all kinds of notifications.

            • Mel Choyce 6:59 pm on January 15, 2014 Permalink

              My vote would be for using the current mentions emails for trac, if only because having an email that’s more designed makes it easier to scan and understand.

              (I’d also be game for redesigning the mention emails.)

    • Pippin Williamson 10:29 pm on January 9, 2014 Permalink | Log in to Reply

      Fantastic!

    • keoshi 10:35 pm on January 9, 2014 Permalink | Log in to Reply

      This is awesome! Great work.

    • Jose Castaneda 10:38 pm on January 9, 2014 Permalink | Log in to Reply

      Awesomesauce!!

    • Amy Hendrix (sabreuse) 10:41 pm on January 9, 2014 Permalink | Log in to Reply

      Yay! I’m really liking all of these improvements. Trac is feeling both friendlier and more usable already.

      I’ve pretty much stopped using ticket cc’s since I sacrificed my inbox to the firehose — what you say about using star counts as votes suggests that it might be worth starring to indicate interest even though I’m already notified?

    • Kim Parsell 11:14 pm on January 9, 2014 Permalink | Log in to Reply

      Thank you for all of the hard work – Trac is so much better to work with now. :)

      I’ve got several tickets from a few years ago with CCs for an old email address that will be going away soon. Do you want to remove those CCs so you don’t get the email bounces from them, if there would be any activity on them?

      • Andrew Nacin 11:28 pm on January 9, 2014 Permalink | Log in to Reply

        I managed to map every email address CC to the corresponding user account (took me about five hours to script it then manually go through about 200 unknowns). I then imported all of them into the new notifications system, which is based on usernames. Then I literally emptied every CC field across all tickets. So those old tickets are now linked to directly to your username and profile email. :-)

    • Brad Touesnard 12:33 am on January 10, 2014 Permalink | Log in to Reply

      Well done sir!

    • Lance Willett 3:16 am on January 10, 2014 Permalink | Log in to Reply

      This is really cool—makes Trac even more user friendly and useful for both newbies and veterans alike.

    • Rami Yushuvaev 9:29 am on January 10, 2014 Permalink | Log in to Reply

      Looks great!

    • Marcus 11:50 am on January 10, 2014 Permalink | Log in to Reply

      bravo, removing cc is a welcome feature :)

    • Lance Willett 6:42 pm on January 10, 2014 Permalink | Log in to Reply

      @nacin — Got my first email for Bundled Themes, nice! It was a notification for #26782.

      One question: I noticed the Reply-To is wp-hackers@lists.automattic.com — is that still correct?

      • Andrew Nacin 6:48 pm on January 10, 2014 Permalink | Log in to Reply

        Yeah, Trac emails have always been set up for that. Mostly to prevent replies from trying to go to the wp-trac list, which is announcement only.

        • Lance Willett 6:51 pm on January 10, 2014 Permalink | Log in to Reply

          Cool.

        • Lance Willett 6:51 pm on January 10, 2014 Permalink | Log in to Reply

          Crazy idea: what if an email reply made a new comment on the ticket?

          • Helen Hou-Sandi 6:56 pm on January 10, 2014 Permalink | Log in to Reply

            I thought about asking about that once, and then thought about how often using that feature on Basecamp leads to forgetting and/or ignoring context. It’s also good to actually visit tickets sometimes, as not everybody knows that uploading attachments doesn’t trigger any notifications.

            • Andrew Nacin 7:09 pm on January 10, 2014 Permalink

              GitHub supports this and every once in a while you can tell someone used it because some quoted text sneaks in. I think that ultimately, it’s a pretty edge feature. The cost of implementing this far outweighs the limited benefit.

              It doesn’t look like there are any decent Trac plugins that implement this. Even if they did, it sounds like it’d be a nightmare even just to configure and get working. It’s still a cool idea, of course, just not worth the energy.

    • Robert Dall 6:59 pm on January 10, 2014 Permalink | Log in to Reply

      @nacin When you click on next ticket… It stays in the same report now! Cool!!!

  • Andrew Nacin 7:47 pm on January 6, 2014 Permalink | Log in to leave a Comment
    Tags:   

    Lots of improvements to Core Trac 

    Over the holidays, I was sick for two weeks (boo). While resting and recovering, I took to working on our tools (yay). Specifically, Trac. Here’s an overview of the added features, enhancements, and bug fixes. There’s a lot (and more on the way), so I’ll try to keep each point brief. If you have any questions I’d be happy to elaborate in the comments.
    (More …)

     
  • Andrew Nacin 7:51 pm on January 3, 2014 Permalink | Log in to leave a Comment
    Tags: ,   

    The email used for notifications on Trac is currently specified in Trac preferences. I’m changing this to pull automatically from your WordPress.org profile. After this change, there will not be a way to have a separate email address for Trac and any manually specified email address will be overridden.

    We need to make this change because, very simply, it’s a better user experience. By killing this extra user preference, it’s one less step users need to set up in order to receive notifications. (And a tighter feedback loop could possibly boost engagement.)

    That will affect about a thousand users we have in the system, probably incidentally for most. But, about 50-100 users might have been deliberately declaring a separate email address; for example 50 users had “trac” appear specifically in their email. Even without a dedicated email, it is trivial to filter Trac emails; you just might need to make some adjustments.

    As you may have noticed, this is part of a series of changes I’ve been making to Trac. I’ll be doing a summary post in a few days outlining everything that has changed.

    NOTE: This does not affect wp-trac@lists.automattic.com. If you subscribe to the “firehose” this is not affected.

    Edit, 20:23 UTC. This is now enabled. For the moment, the email address will sync when you next visit Trac. Please find me if you have any questions.

     
    • Ionel Roiban 7:55 pm on January 3, 2014 Permalink | Log in to Reply

      Sounds good.

    • Ryan Duff 7:57 pm on January 3, 2014 Permalink | Log in to Reply

      Is there an ETA on the time this will be changed?

      I’ll have to update my email filters on my .org acct and would like to have it ready to test so my phone doesn’t blow up some night when I’m sleeping.

      That’s why I used a specific “list” acct ;)

      • Ryan Duff 7:59 pm on January 3, 2014 Permalink | Log in to Reply

        Err wait, this is the notification email, not the list email. I may be fine as I think that goes to the same email on my .org profile already.

        Heads up may still be nice for the handful that will have to adjust though.

      • Andrew Nacin 8:00 pm on January 3, 2014 Permalink | Log in to Reply

        Sometime in the next 1-72 hours.

        Note — you have the same email address in both Trac references and your WordPress.org profile. If you were referring to the firehose mailing list, that’s not affected. I’ve added a note to the bottom of the post.

        • Ryan Duff 8:13 pm on January 3, 2014 Permalink | Log in to Reply

          Yup. I’m using a separate address for the svn/trac mailing lists. When I went to verify email filters I realized that the ones that come through as trac notification went to the email on file for .org.

          This week has been a bumble from the start. At least it’s Friday. Thanks for following up!

    • Andrew Nacin 8:08 pm on January 3, 2014 Permalink | Log in to Reply

      Here is a short list of names I recognize who this will affect: @westi @dd32 @viper007bond @sterlo @coffee2code @chmac @eddiemoya @kawauso @lloydde @mrmist @ptahdunbar.

      In most cases, though, the user updated their WordPress profile but never bothered to update Trac. That’s why we’re doing this!

    • Eddie Moya 7:38 am on January 4, 2014 Permalink | Log in to Reply

      I did use “+trac” in my email address to filter for it, so I really appreciate that you went through the effort of figuring out who might specifically affected by it. Otherwise I would have been confused about why my filters stopped working, since this is something I haven’t touched in a very long time.

      Idk if the list is just really that short of people affected – but maybe it would make sense to send out a blast email on trac to let everyone know that way in case they aren’t lucky enough to be the 11 names you listed. Just a thought.

      Thanks again for the heads up.

  • Andrew Nacin 9:26 pm on December 31, 2013 Permalink | Log in to leave a Comment
    Tags: ,   

    Commit announcements for 3.9 

    Lots of news to share! First: Helen Hou-Sandí has had guest commit for the past three release cycles. She’s been spending the last year reviewing contributions, mentoring contributors, and working on some of our larger UI projects. I’m proud to announce @helen is now a permanent committer to WordPress!

    We’ve invited John Blackbourn (@johnbillion) to be a committer for the 3.9 cycle. His strong, consistent contributions have been backed by excellent judgment and temperament.

    Matt Thomas, who led the dashboard redesign in 3.8 (and 3.2, and 2.7, etc.), will keep his commit to continue to maintain and improve WordPress UI. He’s been a great mentor to many contributing designers and his long-term impact is indelible.

    For the last few years, we’ve been granting commit access on per-cycle basis, sometimes for a particular component, feature, etc. Generally, after about a year, a guest committer can be considered for permanent commit access. Dominik Schilling, Sergey Biryukov, Drew Jaynes, and Scott Taylor have all had their commit extended for 3.9.

    Drew (@DrewAPicture) was given temporary commit for inline documentation starting with 3.7. He’s been heading up the long-running initiative to document every hook in WordPress. Scott (@wonderboymusic) also started committing during 3.7, and has a particular penchant for digging deep into the query and taxonomy APIs. And Sergey (@SergeyBiryukov) and Dominik (@ocean90), well, they are forces of nature.

    (@aaroncampbell was also given guest commit in 3.7, but he ended up not having much time to use it.)

    Here’s a full list of those with permanent commit: @markjaquith, @ryan, @westi, @matt, @azaozz, @dd32, @koopersmith, @duck_, @helen, and me (@nacin); @lancewillett for bundled themes; @iammattthomas for UI. You might have also seen commits before from @josephscott (XML-RPC), @nbachiyski (internationalization), and @mdawaffe (secret weapon for really tricky problems).

    Next weekly meeting is January 8. Happy new year, everyone. Here’s to a great 2014.

     
  • Andrew Nacin 9:01 pm on December 18, 2013 Permalink
    Tags:   

    Automatic updates for 3.8 started to flow early Monday. Since major releases are opt-in, future ones will get automatic update instructions immediately. It should have happened on release day this time as well (oversight on our part). Minor releases will continue to get a delayed roll-out. 3.7.1 happened over the course of about four days, while 3.8.1 will probably happen over a few hours or a day at most.

    Success rate is around 99.78 percent of about 15,000 updates. That’s a bit lower than 3.7.1, which was 99.988 percent for about a million updates. This is to be expected; for example, far more files were changed. Definitely some great numbers but also lots of room for improvement! I’d love to be able to count the number of critical failures on two hands by the end of 2014.

     
    • Andrew Nacin 9:02 pm on December 18, 2013 Permalink | Log in to Reply

      Remember that a critical failure (what these numbers are calculating) only means “we started to copy files and failed to verify we copied all of them”. It doesn’t mean the site crashed and burned.

    • niklasbr 10:00 pm on December 18, 2013 Permalink | Log in to Reply

      Does 3.8.1 verify that no plugins or themes break and not just the WP update itself works as it ought to?

      • Ipstenu (Mika Epstein) 10:30 pm on December 18, 2013 Permalink | Log in to Reply

        There is no 3.8.1 at this time (we’ve not even started talking about it yet in today’s devchat ;) )

      • Andrew Nacin 11:11 pm on December 18, 2013 Permalink | Log in to Reply

        We’re not concerned about breaking plugins or themes in a minor release. These changes go through a very rigorous review process and are never designed to break or threaten compatibility.

        Major releases, on the other hand, could possibly cause problems on a minority of sites — and yeah, that’s where verifying that plugins and themes are good to go will come into play.

        The updating code is the same for both minor and major releases — shifting major releases to be auto-updated by default is all about avoiding breakage and shifting perceptions.

        • niklasbr 11:27 pm on December 18, 2013 Permalink | Log in to Reply

          Can I somehow convince you that it is a cause for concern or have you made up your mind on the topic?

          Plugins and themes do break all the time in minor releases, however, that is not a big concern when updating manually. However, it becomes a huge issue when they break automatically.

          • John Blackbourn 2:35 am on December 19, 2013 Permalink | Log in to Reply

            Plugins and themes do break all the time in minor releases

            Do you have a list of such plugins and themes (and which releases they broke in) to hand? Or a link to corresponding forum threads? Bear in mind we’re talking about minor releases here, which is a release number with a third level. For example, 3.7 is a major release, 3.7.1 is a minor release.

            • niklasbr 2:12 pm on December 22, 2013 Permalink

              I don’t have a list because I never thought such a list has been needed.

              WP Super Cache and JetPack are probably the most prominent I have experienced issues with in some minor releases (e.g. 3.6 to 3.6.1 and 3.5 to 3.5.1 and 3.3.1 to 3.3.2).

          • Jeff Chandler 3:23 am on December 19, 2013 Permalink | Log in to Reply

            The topic and concerns were discussed ad nauseum around the time the feature was added to WordPress http://make.wordpress.org/core/2013/10/25/the-definitive-guide-to-disabling-auto-updates-in-wordpress-3-7/ all of the angles have been discussed and no convincing is necessary. The only convincing that can happen at this point is from the data that is gathered from sites performing auto updates.

    • GJudy 4:50 pm on December 27, 2013 Permalink | Log in to Reply

      I am an intermediate user. I have about 30 sites, but only one seems to be doing this auto-update for the minor changes. It sent me 6 emails over three days that the update was in progress and done. http://gentwo.sensiblestrategy.com says 3.8.1 alpha is the version running. I don’t know what I did to get this one site into that update queue.

      None of my other sites have done anything. They were updated about the same time to version 3.8. Many were done using ftp because they were behind more than one version. Some were done with the update button even though they were behind more than one version. I turned all plugins off before updating exept on one site. That little test indicated deactivating may not have been necessary. One theme gave some trouble. Some appeared to have updated automatically (and silently) from 3.7 to 3.7 1, and I used the update now option successfully. There were some that get more regular attention and plugins, themes and WP were properly updated. There was one that had a glitch and I had to re-ftp the wp-admin. A couple were more balky about the cute dashboard choices.

      Nothing appears to be wrong with the site that had the auto-update to 3.7.1 and later to 3.8.1, except for the many notifications.. Not a profound issue or question, but I appreciate any input. Thank you.

      • John Blackbourn 4:54 pm on December 27, 2013 Permalink | Log in to Reply

        Best thing to do in this case is to install the Background Update Tester plugin on each of the affected sites and see what it reports. It should help you pinpoint any problems.

      • Ipstenu (Mika Epstein) 9:06 pm on December 27, 2013 Permalink | Log in to Reply

        3.8.1 alpha means you’re on the development version of WP (seeing as … 3.8.1 isn’t released yet). If you don’t want to be, manually reinstall the 3.8 files from a fresh copy and make sure you don’t have a beta tester plugin installed.

        More help needed? This would be time to post in the support forums :)

    • iburketh 1:25 am on January 7, 2014 Permalink | Log in to Reply

      Since this upgrade took place I can no No longer access my administrative screen in WordPress. I do have an error message that says word press update available. Update to three point eight – 26 now. However when I click update in my server I get another error message saying no update available. I have tried almost every solution available in the forums to no avail. Any help you could offer would be appreciated. I’m at a point where I am considering the reinstallation of Word press. However most of the posts that I’ve read say this is not necessary. Your thoughts would be appreciated. Thanks

      • Dion Hulse 1:43 am on January 7, 2014 Permalink | Log in to Reply

        If you’re using an external Object Cache (such as Memcache, APC, WinCache, etc) then try forcing that to reload, it may have become out of sync with your database.
        Failing that, on the Updates page (under Dashboard) try hitting Re-install and see if that helps.

    • webado 12:32 pm on January 24, 2014 Permalink | Log in to Reply

      I just got a first ever notification of an automatic update to 3.8.1. Kind of scared me, but as it happens things are OK.

      My concern is about WP sites which might have been hacked. None of mine have, but I deal with many such sites very often. Updating WP is always one of the things to do but first one must find and clean up the hack. What happens when there is an automatic update? Couldn’t this create more trouble potentially?

    • brasscrest 1:46 pm on January 24, 2014 Permalink | Log in to Reply

      Quote from the update page (when you don’t have automatic updates on):

      “Important: before updating, please back up your database and files. For help with updates, visit the Updating WordPress Codex page.”

      That’s why I don’t want automatic updates on. Because I actually back up my sites before implementing every core update. And also before updating most plugins. While I appreciate the hard work of the core team and alpha/beta testers, I have spent many thousands of hours developing content over 14 years. I’m not going to compromise that work by turning on background auto-updates that happen without giving me a chance to take a backup.

  • Andrew Nacin 8:25 pm on December 18, 2013 Permalink
    Tags:   

    Proposed agenda for today’s dev chat (~35 minutes from now):

    • Initial debrief for 3.8. Start to think about what we can learn from 3.8.
    • Status report of bugs/regressions for 3.8.1. Look at a tentative 3.8.1 timeline.
    • Initial 3.9 discussion. Discuss potential feature plugin candidates, timeline, and release leads.
     
  • Andrew Nacin 5:58 pm on November 13, 2013 Permalink
    Tags:   

    Proposed coding standards change (always require braces) 

    Our current PHP coding standards specify that for if statements, “single line blocks can omit braces for brevity.” All other constructs “should always contain braces as this enhances readability, and allows for fewer line edits for debugging or additional functionality later.” I’d like to propose we always require braces for all blocks.

    One could argue about coding standards all day (not in this comments thread), but allowing braces to be omitted is probably the only thing in our standards that can cause legitimate problems, like making it easier to introduce errors. It also makes code more annoying to patch and less maintainable, and requires larger diffs. And, it’s more work to debug code, as you have to add braces to add debugging lines, then remove them when done. It’s easy to attempt to add a new line inside the conditional and screw it up, which has occurred even a few times in the last few weeks. For example:

    if ( some_conditional() )
        some_new_line();
        return some_function();
    

    Another catalyst here is our new JavaScript standards. Due to multi-line chaining and other situations, it is basically insane to omit curly braces in JavaScript. It would be nice if our standards matched, making this the right time to raise the issue for PHP. Our coding standards have a general emphasis on whitespace and readability, and the lack of braces do generally make code feel lighter and easier to scan, but I think we’ve reached a breaking point. I’ve surveyed the other lead developers and the consensus was that while omitting braces is elegant, it doesn’t best serve the needs of the project (with many people editing many things often).

    I’d like to discuss this during today’s meeting, and ideally make a decision.

    I will mention that adopting this will mean all new code will receive braces. We can discuss what to do with existing code. Please don’t waste any time on a patch to add braces manually; we’d script it and do it in one commit if that’s the route we decide to take. (There’s more than 8,000 structures lacking braces.) Also worth reading in preparation for a discussion is a section in Linux’s contributor documentation on coding style pitfalls, something I’ve linked to before.

    P.S. This wouldn’t change bracing placement. It’s still like this:

    function func_name() {
        if ( some_conditional() ) {
            echo "Braces shall be 'cuddled,' not pushed to the next line."
        } else {
            echo "For all control structures.";
        }
    }
    

    Update: This proposal was accepted during the Wednesday meeting. I’ve updated the coding standards handbook page. We’ve yet to discuss what we’ll do for old code, but all new and changed code should receive braces.

     
    • Bryan Petty 6:09 pm on November 13, 2013 Permalink

      Just thought I’d point out that this would make our control structure guidelines PSR-2 compatible, not that it should be a deciding factor, but it certainly doesn’t hurt.

      • Ryan McCue 2:13 am on November 14, 2013 Permalink

        We’re still incompatible in numerous other ways, like: tabs instead of spaces (yay tabs!), opening braces for functions/etc being on the same line (yay!) and my old favourite, spaces everywhere (BOOOO). :)

    • Weston Ruter 6:21 pm on November 13, 2013 Permalink

      +1

      Filed a issue to add this to the PHP_CodeSniffer rules: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/124

      By the way, we’ve been working on a script for PHP_CodeSniffer which will only apply the rules to files and the lines in these files that have changed, that is, it will only report errors on uncommitted code. This would make PHP_CodeSniffer suitable for use in WordPress Core, as it would guide developers to apply the rules for new code but leave old code alone. The script is called phpcs-patch: https://github.com/x-team/PHP_CodeSniffer/blob/phpcs-patch/scripts/phpcs-patch

    • nofearinc 6:28 pm on November 13, 2013 Permalink

      That’s incredible improvement given the hundreds of issues with the copy-paste of a second statement added to an if clause getting executed outside.

      As a reminder, we’ve been adhering to this in Java for a long time – http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449

    • Scott Taylor 6:31 pm on November 13, 2013 Permalink

      I’d grown to like bracelessness, but ok. #thanksobama

      • Mike Schroder 9:55 pm on November 13, 2013 Permalink

        Yeah. I tend to agree that some statements *look* better without the braces.

        But, in terms of core, it seems safer to begin requiring them — at the very least for new code.

    • Peter Chester 6:36 pm on November 13, 2013 Permalink

      +1 I’ve always felt that explicit braces improve legibility.

    • Ian Dunn 6:40 pm on November 13, 2013 Permalink

      +1 because of consistency (internally and with the JS guide), and not having to add braces for debugging and then remove them.

    • Beau Lebens 6:47 pm on November 13, 2013 Permalink

      I’d definitely agree with this. My 2 biggest reasons would be consistency with JS, and ability to add/remove debug info easily without introducing bugs.

    • Tom Auger 6:47 pm on November 13, 2013 Permalink

      +1 even though I like the no braces construct myself, I definitely agree for all the reasons Nacin lists.

      • Tom Auger 6:50 pm on November 13, 2013 Permalink

        Although, what about allowing 1-liners, like:
        if ( $braceless_condition ) echo “1 – liner only”;

      • Franz Josef Kaiser 9:13 pm on November 13, 2013 Permalink

        And what about one-liners like this one?

        defined( 'ABSPATH' ) OR exit;

      • Andrew Nacin 11:06 pm on November 13, 2013 Permalink

        But this whole thing was about taking one-line conditions and always requiring braces. One line, no line break was against our current standards and would continue to be against these.

        A construct like defined( 'WPINC' ) or exit; remains OK — of course, in limited use, pretty much only for this right here.

    • Stefano Aglietti 6:55 pm on November 13, 2013 Permalink

      IN my day developping of theme plugins etc I qlways add braces even for single line i prefere, less problem if code evolves and editor i use (PHPstorm show better the code and it show up clearer. +1 to Nacin proposal

    • Marcus 7:00 pm on November 13, 2013 Permalink

      Yes please!

      Lack of braces is one thing I really dislike about the WP coding structure but have learnt to live with.

      I use braces by habit and aside from easier readability, it avoids copy/paste errors or editing code. Would love to see that practice in core too!

      As Tom Auger suggested, I think one liners are acceptable and do use this approach these days, mostly thanks to the code in core :)

    • Ryan Hellyer 7:16 pm on November 13, 2013 Permalink

      I recall finding conditionals very confusing when I first started out. I immediately grok’d how braces worked, but “if ( bla == blob ) : ” confused me, and omitting braces or and endif REALLY confused me.

      I’ve grown to not care about this issue, but I suspect I’m not the only person who got flummoxed by this when they first started out.

      I started using braces for (almost) everything about a year ago, but had been omitting them for a long time as I thought they were the recommended practice since I saw them in core all the time.

    • Per Soderlind 7:27 pm on November 13, 2013 Permalink

      +1, I allways use braces, been using them since I learned C in the 80ies, even for 1-liners

    • Doug Wollison 7:32 pm on November 13, 2013 Permalink

      +1, especially on the consistency with JS angle; might as well get them as close as possible.

    • Jeff Rose 7:32 pm on November 13, 2013 Permalink

      +1 I’m a relative newb, but I much prefer the explicit braces and consistency

    • Paul Gibbs 8:11 pm on November 13, 2013 Permalink

      I support leaving the code standards as they are. I prefer leaving braces out on short, simple, and uncomplicated IFs because there’s less visual noise to read when you are scanning code; it makes the code more readable, as is briefly alluded to in the post.

      It is always possible for accidents to occur when writing or reviewing code; without leaning too heavily on the example given, any codebase will still occasionally receive other similar, easily-overlooked errors. Requiring braces will just change the type of problems and their consequences. I don’t see a net win here.

      Using IDEs for development can also make it easier to debug or inspect values at certain points in execution with breakpoints. It’s something I am trying to do more often in my own work, because it stops me making any changes to the codebase that I’m rushing through, perhaps making quick and messy changes for debugging! Sometimes a debugger is a better tool than print-based debugging, though of course, the inverse is also true.

      • Ryan McCue 2:15 am on November 14, 2013 Permalink

        One of the other reasons it’s nice is that it reduces noise in patches. Adding a line to a single line if means changing 3 lines, which is visual noise when you’re reviewing. I like excluding them occasionally, but the diff noise is what ruins it for me.

    • Joan Boluda 8:21 pm on November 13, 2013 Permalink

      +1 because of consistency

    • John Blackbourn (johnbillion) 8:44 pm on November 13, 2013 Permalink

      I’d like to discuss this during today’s meeting

      I think we have better things to discuss. Please just do it and save everyone a mostly pointless conversation.

      • mattyrob 9:43 pm on November 13, 2013 Permalink

        +1 to that ;) And the original idea too.

    • Matt van Andel 9:17 pm on November 13, 2013 Permalink

      I have but one thing to say about this…

      YES

    • Lance Willett 9:50 pm on November 13, 2013 Permalink

      +1 even though I’m used to it, the reasons you mentioned outweigh the habitual.

    • Chip Bennett 10:41 pm on November 13, 2013 Permalink

      +1, which, coming from the resident pedant, should come as no surprise. :)

    • Mike Schinkel 10:42 pm on November 13, 2013 Permalink

      Like Matt van Andel said:

      YES!

      This will make it possible to set breakpoints for debugging on all lines of code with PhpStorm (and other PHP IDE debuggers.)

      And if you are looking for the first candidate for this, please update template-loader.php with this new standard, or bless the task and I’ll do it.

    • Arnan de Gans 11:04 pm on November 13, 2013 Permalink

      I agree with this idea :) plugin and theme makers should do this also.

    • Robert Chapin 12:43 am on November 14, 2013 Permalink

      Braceless ifs always bothered me, not because of inserting extra lines, but because PHP also supports two braceless ‘else’ syntaxes that confuse the hell out of me.

    • Brad Davis 12:45 am on November 14, 2013 Permalink

      +1.

    • jarednova 1:37 am on November 14, 2013 Permalink

      +1. Braceless-ness is one of those “timesaving” things that actually costs more time. Clarity > Brevity

    • photocrati 3:06 am on November 14, 2013 Permalink

      You’re just kickin’ in an open door with this suggestion …

    • Edward Caissie 3:11 am on November 14, 2013 Permalink

      As my alter-ego said … you’re kickin’ in an open door with this suggestion.

    • Andrew Nacin 4:21 am on November 14, 2013 Permalink

      This proposal was accepted during the Wednesday meeting. I’ve updated the coding standards handbook page. We’ve yet to discuss what we’ll do for old code, but all new and changed code should receive braces.

    • Nashwan Doaqan 11:01 am on November 14, 2013 Permalink

      +1 even though I’m used to it, I will update all my plugins code :)

    • Tom McFarlin 1:58 pm on November 14, 2013 Permalink

      +1.

    • contempoinc 6:12 pm on November 14, 2013 Permalink

      +1

    • Danny van Kooten 6:33 pm on November 14, 2013 Permalink

      +1

    • Barry Kooij 6:48 pm on November 14, 2013 Permalink

      +1

    • dimitrov.adrian 8:19 am on November 15, 2013 Permalink

      This is good approach, but I think that this is right time to suggest some more things. It’s about the space usage.

      my_function( $param1, func_param( $param2 ) );

      why just not

      my_function($param1, func_param($param2));

      IMO in most cases this make code more hard reading

      Also, I think using of predefined constats (true, false and null) should be in uppercase – TRUE, FALSE and NULL

      In this order I think that code standards of Drupal (https://drupal.org/coding-standards) can help here, as they try to follow PEAR code standards.

      • Christian Foellmann 10:40 am on November 15, 2013 Permalink

        I dont think we should change one of the most basic conventions of WP coding standards.

      • Andrew Nacin 1:50 pm on November 15, 2013 Permalink

        Sorry, I can’t ever see these rules being changed. Long live whitespace. Let the code breathe. :-)

    • Christian Foellmann 10:36 am on November 15, 2013 Permalink

      +1 for “always require braces”

    • Gregory Karpinsky 12:10 pm on November 15, 2013 Permalink

      +100000!!! Finally!

  • Andrew Nacin 7:14 pm on November 1, 2013 Permalink
    Tags: ,   

    @matt will be running a WordPress 3.8 feature planning/decisions meeting on Monday, November 4, 21:00 UTC. Now that Daylight Saving Time has ended for both Europe and the U.S., note that the weekly Wednesday meeting is now moved from 20:00 UTC to 21:00 UTC (4 p.m. EST, 1 p.m. PST). (Americans, change your clocks on Sunday.)

    I’d strongly encourage everyone to study, test, and weigh in on the four 3.8 proposals before the meeting. I believe the goal of the meeting will be to establish what exactly gets merged into 3.8.

    API enhancements, bug fixes, etc. can/will continue as usual — it would be awesome if we had a surge not unlike 3.7. But for now, 3.7.1 is out, so stare at the download counter sip some tea, and relax this the weekend. :)

     
c
compose new post
j
next post/next comment
k
previous post/previous comment
r
reply
e
edit
o
show/hide comments
t
go to top
l
go to login
h
show/hide help
shift + esc
cancel