I have spent quite a few days working on a two character bug fix for the
dartdoctool, 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.gitI add that repository to my PATH:
PATH=$HOME/repos/depot_tools:$PATHNext, 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 runhooksThen, 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 runhooksAfter the
gclient synccommand and after
runhooks, I have several files lying around:
dart git:(master) ✗ gst # On branch master # Untracked files: # (use "git addThose 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:
..." 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)
➜ dart git:(master) ✗ git status # On branch master # Changes to be committed: # (use "git reset HEADI make sure that my new test passes for me:
..." to unstage) # # modified: sdk/lib/_internal/dartdoc/lib/src/dartdoc/comment_map.dart # new file: sdk/lib/_internal/dartdoc/test/comment_map_test.dart
➜ 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-successThen I “break” the
importstatement for the
unittest.dartlibrary 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.dartNext, 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) [firstname.lastname@example.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.dartThat 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.