Add Crossdart links #856

astashov · 2015-08-24T18:02:50Z

Added them to the _source_code partial, so every time we show the source code for something, it will have the Crossdart link.

/cc @sethladd

googlebot · 2015-08-24T18:02:51Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

astashov · 2015-08-24T18:04:19Z

I signed it!

googlebot · 2015-08-24T18:04:20Z

CLAs look good, thanks!

sethladd · 2015-08-24T18:21:07Z

Cool!

Can you add a command-line option to toggle this?

astashov · 2015-08-24T18:21:37Z

Sure!

sethladd · 2015-08-24T19:59:56Z

lib/src/model.dart

we would like this to be https or relative (//www.crossdart)

Will do //www.crossdart, thanks

can you host crossdart on SSL?

Sure, I host it on Google Cloud Storage (it's a static site), so I just need to buy SSL certificate, that's it.

Unfortunately, google cloud storage doesn't allow for custom certs + custom DNS :(

You could put cloudfront in front.

Oh really? Okay, I need to look into it then. Thanks for advice.

astashov · 2015-08-28T15:41:48Z

Okay, so, crossdart.info now supports HTTPS (https://crossdart.info). Unfortunately, I couldn't figure out how to make it work with CloudFront and GCS, so I moved everything to S3+CloudFront+Route53.

Also, added a CLI flag to turn Crossdart on.

@sethladd, PTAL.

sethladd · 2015-08-28T21:46:20Z

Hi!

OK, we got our build in order and tests should be passing on our buildbots. Please take a moment to rebase and send up a new diff. Thanks!

sethladd · 2015-08-28T21:47:06Z

lib/src/cache.dart

Can you use a type annotation here? top-level variables should be typed. Thanks!

astashov · 2015-08-29T13:24:34Z

Rebased. It's weird - Travis CI complains that I have to run dartfmt -w lib/src/model.dart, but when I run it, it doesn't make any changes. Any idea why? Maybe I miss some dartfmt config file or something like that?

devoncarew · 2015-08-29T13:58:12Z

Travis is running 1.12.0-dev.5.10 for that build. I'm guessing that the dev formatter produces slightly different code than the stable formatter. Are you on stable? That could be it -

sethladd · 2015-08-31T15:56:05Z

What @devoncarew said. dartfmt changed its style between releases.

Now that 1.12 is out, we'll turn on stable-channel testing again and everyone can and should be using 1.12's dartfmt.

Sorry for the issues!

astashov · 2015-08-31T15:57:24Z

Yup, just noticed, already upgrading our projects to Dart 1.12.0. Null-aware operators FTW! :)
Will check this one out tonight.

astashov · 2015-09-01T16:03:58Z

Okay, Travis is now green, I squashed everything into one commit, and ready to go! :)

sethladd · 2015-09-01T18:36:33Z

lib/src/model.dart

Sorry I missed this before. Can you add some tests for this?

We don't need to test the generated HTML but it would be nice to test the output of calling element.documentation and element.documentationAsHtml with this option on and off.

Makes sense, will add.

Thanks. Apologies for not catching that before.

No problem :) Added tests, PTAL.

sethladd · 2015-09-02T16:06:58Z

Nice! lgtm once we fix the failing test on windows.

Actually, can you fix the test failing on windows?

00:10 +135 ~1 -1: Method .crossdartHtmlTag() it returns a Crossdart link when Crossdart suppo
    rt is enabled
    Expected: contains '//crossdart.info/p/test_package/0.0.1'
      Actual: '<a class=\'crossdart\' href=\'/C:/projects/dartdoc/test_package/lib/example.dart.html#line-73\'>Link to Cro
    ssdart</a>'

    package:test                expect
    test\model_test.dart 860:9  main.<fn>.<fn>.<fn>

astashov · 2015-09-02T16:59:00Z

Okay, let me try that.

astashov · 2015-09-03T16:15:05Z

All green now, PTAL, @sethladd!

astashov · 2015-09-15T01:33:18Z

Ping? :)

sethladd · 2015-09-15T16:42:03Z

Ack, sorry I missed this. Looks like you need to rebase. And then we'll merge it.

sethladd · 2015-09-29T21:45:16Z

Friendly ping. Care to rebase this so we can merge? Thanks!

Added them to the _source_code partial, so every time we show the source code for something, it will have the Crossdart link. Also, added --add-crossdart cli option. Unfortunately, I couldn't find any global config entity in DartDoc, and passing addCrossdart bool through all the code down to SourceCodeMixin seems to be too dirty and awkward. So I created the Config class, which for now contains only one field (addCrossdart), and use that class in SourceCodeMixin to decide whether I should add crossdart url or not. The protocol and domain for crossdart link is '//', so it'd match the currently used protocol.

astashov · 2015-09-29T22:20:07Z

Rebased! Sorry for delay.

Add Crossdart links

sethladd · 2015-09-29T22:21:04Z

Super cool, thanks!

sethladd reviewed Aug 24, 2015
View reviewed changes

sethladd reviewed Aug 28, 2015
View reviewed changes

lib/src/cache.dart Outdated

Copy link

Contributor

sethladd Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use a type annotation here? top-level variables should be typed. Thanks!

astashov force-pushed the add-crossdart-links branch from 573ab0a to de1bffe Compare August 29, 2015 01:58

astashov force-pushed the add-crossdart-links branch 2 times, most recently from d1ef0ea to 74c8aad Compare September 1, 2015 14:18

astashov force-pushed the add-crossdart-links branch 3 times, most recently from 271830e to a911554 Compare September 1, 2015 18:07

sethladd reviewed Sep 1, 2015
View reviewed changes

astashov force-pushed the add-crossdart-links branch from bfc8265 to 617fd09 Compare September 2, 2015 16:03

astashov force-pushed the add-crossdart-links branch 6 times, most recently from f38ebd1 to 9ffbfef Compare September 3, 2015 14:42

astashov force-pushed the add-crossdart-links branch 2 times, most recently from f41c916 to 46229a6 Compare September 3, 2015 15:23

astashov force-pushed the add-crossdart-links branch from 46229a6 to 904bf07 Compare September 29, 2015 22:14

sethladd added a commit that referenced this pull request Sep 29, 2015

Merge pull request #856 from astashov/add-crossdart-links

595b7e6

Add Crossdart links

sethladd merged commit 595b7e6 into dart-lang:master Sep 29, 2015

This was referenced Sep 29, 2015

Link to crossdart from docs #933

Closed

publish 0.7.0 #935

Merged

Add Crossdart links #856

Add Crossdart links #856

Uh oh!

Conversation

astashov commented Aug 24, 2015

Uh oh!

googlebot commented Aug 24, 2015

Uh oh!

astashov commented Aug 24, 2015

Uh oh!

googlebot commented Aug 24, 2015

Uh oh!

sethladd commented Aug 24, 2015

Uh oh!

astashov commented Aug 24, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astashov commented Aug 28, 2015

Uh oh!

sethladd commented Aug 28, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astashov commented Aug 29, 2015

Uh oh!

devoncarew commented Aug 29, 2015

Uh oh!

sethladd commented Aug 31, 2015

Uh oh!

astashov commented Aug 31, 2015

Uh oh!

astashov commented Sep 1, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sethladd commented Sep 2, 2015

Uh oh!

astashov commented Sep 2, 2015

Uh oh!

astashov commented Sep 3, 2015

Uh oh!

astashov commented Sep 15, 2015

Uh oh!

sethladd commented Sep 15, 2015

Uh oh!

sethladd commented Sep 29, 2015

Uh oh!

astashov commented Sep 29, 2015

Uh oh!

sethladd commented Sep 29, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants