Submitting a Code Fix for the Dart SDK

‹prev | My Chain | next›

I have spent quite a few days working on a two character bug fix for the dartdoc tool, which is part of the Dart SDK. Actually, I found the fix on the first day, but had me a good old time rooting through the code and figuring out how to test it. But finally, I am ready to submit my code to the the Dart project.

I signed the Google Individual Contributor License Agreement yesterday.

Next, I need to install the “depot tools”, which are used for code review submissions:

➜  repos  git clone https://git.chromium.org/chromium/tools/depot_tools.git

I add that repository to my PATH:

PATH=$HOME/repos/depot_tools:$PATH

Next, I have to grab the Dart source. I cloned the GitHub mirror the other day, but it seems like I need the “real” source for this. Since the contribution instructions refer to using git, I will follow along with git section of the “getting the source“ documentation:

➜  repos  svn ls http://dart.googlecode.com/svn/branches/bleeding_edge/
AUTHORS
LICENSE
PATENTS
dart/
deps/
➜  repos  mkdir dart-repo
➜  repos  cd dart-repo
➜  dart-repo  gclient config http://dart.googlecode.com/svn/branches/bleeding_edge/deps/all.deps
➜  dart-repo  git svn clone -rHEAD http://dart.googlecode.com/svn/branches/bleeding_edge/dart dart
➜  dart-repo  gclient sync
➜  dart-repo  gclient runhooks

Then, I follow the get-latest-changes procedure (presumably I don't really need this since this is newly initialized):

➜  dart-repo  cd dart
➜  dart git:(master) git svn fetch
➜  dart git:(master) git merge git-svn
➜  dart git:(master) gclient sync
➜  dart git:(master) ✗ gclient runhooks

After the gclient sync command and after runhooks, I have several files lying around:

dart git:(master) ✗ gst
# On branch master
# Untracked files:
#   (use "git add ..." to include in what will be committed)
#
#       compiler/dart_analyzer.target.mk
#       dart2js_bot.target.mk
#       samples.target.mk
#       samples/sample_extension/sample_extension.Makefile
#       samples/sample_extension/sample_extension.target.mk
nothing added to commit but untracked files present (use "git add" to track)

Those do not seem to have any effect on things so I get started on my changes. It seems that no one has fixed my newline fix yet, so I add the change to my repository:

➜  dart git:(master) ✗ git status
# On branch master
# Changes to be committed:
#   (use "git reset HEAD ..." to unstage)
#
#       modified:   sdk/lib/_internal/dartdoc/lib/src/dartdoc/comment_map.dart
#       new file:   sdk/lib/_internal/dartdoc/test/comment_map_test.dart

I make sure that my new test passes for me:

➜  dart git:(master) ✗ ./tools/testing/bin/linux/dart sdk/lib/_internal/dartdoc/test/comment_map_test.dart
unittest-suite-wait-for-done
PASS: triple slashed comments retain newlines

All 1 tests passed.
unittest-suite-success

Then I “break” the import statement for the unittest.dart library to conform to the same relative path that the other tests in the same directory use. I still have not figured out an explanation for the different paths, but hopefully this will work on the Dart build system. I commit the fix:

➜  dart git:(master) ✗ git co -b code-samples-for-triple-slash-dartdoc
M       sdk/lib/_internal/dartdoc/lib/src/dartdoc/comment_map.dart
A       sdk/lib/_internal/dartdoc/test/comment_map_test.dart
Switched to a new branch 'code-samples-for-triple-slash-dartdoc'
➜  dart git:(code-samples-for-triple-slash-dartdoc) ✗ git ci -m "Fix for triple-slash dartdocs    
dquote> 
dquote> They had been ignoring multiline markdown such as code samples."


They had been ignoring multiline markdown such as code samples."[code-samples-for-triple-slash-dartdoc 1093824] Fix for triple-slash dartdocs
 2 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 sdk/lib/_internal/dartdoc/test/comment_map_test.dart

Next, I create myself an account on http://chromiumcodereview.appspot.com/.

At this point, I am ready to begin my “change list”. I configure the process with git cl config:

➜  dart git:(code-samples-for-triple-slash-dartdoc) ✗ git cl config
Rietveld server (host[:port]) [https://codereview.chromium.org/]: http://chromiumcodereview.appspot.com
CC list ("x" to clear) [reviews@dartlang.org]:
Tree status URL:
ViewVC URL ("x" to clear) [https://code.google.com/p/dart/source/detail?r=]:

Now, I start the upload process:

➜  dart git:(code-samples-for-triple-slash-dartdoc) ✗ git cl upload
Using 50% similarity for rename/copy detection. Override with --similarity.
Running presubmit upload checks ...

Presubmit checks passed.
 .../dartdoc/lib/src/dartdoc/comment_map.dart       |    2 +-
 .../_internal/dartdoc/test/comment_map_test.dart   |   35 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

That pops up an editor asking for a description of the patch. It is pre-populated with the commit message, but I add a bit more:

# Enter a description of the change.
# This will displayed on the codereview site.
# The first line will also be used as the subject of the review.
Fix for triple-slash dartdocs

They had been ignoring multiline markdown such as code samples.
More information available at:
http://japhr.blogspot.com/2012/11/troubleshooting-triple-slash-dartdocs.html

This also includes a new test file, though I am unsure if the 
paths are correct.

BUG=

After saving that, I am prompted for the password for my account. It takes me a while to figure out that I need to generate an application specific password as described in http://support.google.com/accounts/bin/answer.py?hl=en&answer=185833. The prompt kept telling me to do this, but for some reason I did not register that I need to go through the generation process.

Once I sorted out my password issue, I was able to submit the “change list”, which returned:

Issue created. URL: https://codereview.chromium.org/11358134
Uploading base file for sdk/lib/_internal/dartdoc/lib/src/dartdoc/comment_map.dart
Uploading base file for sdk/lib/_internal/dartdoc/test/comment_map_test.dart

That change list is, indeed, available at: https://codereview.chromium.org/11358134.

Armed with that URL, I can now file a bug report at http://dartbug.com: http://code.google.com/p/dart/issues/detail?id=6584. I am unsure how to attach the “change list” to the Dart bug, so I add it in a comment.

And that should do it. I am reasonably confident that the actual fix is solid. I am less sure about the test (paths to libraries and style). Hopefully I have completed all of the necessary steps correctly.


Day #562