Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore(docs-gen): create plnkr examples with the correct version #15358

Merged
merged 1 commit into from
Nov 5, 2016

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Nov 3, 2016

What is the current behavior? (You can also link to an open issue here)
Currently, plnkrs are opened with the cdnVersion of the built docs app, which means
that snapshot plunkers might not work because they use the cdn (stable version).

Please check if the PR fulfills these requirements

Other information:

Should we update this info for the other deployments too?

@Narretz Narretz added this to the 1.5.x milestone Nov 3, 2016
@Narretz Narretz force-pushed the chore-docs-plnkr-version branch from 79eb5e4 to 0bd2604 Compare November 3, 2016 13:49
// might not work (possibly in subtle ways).
var examplesCdnUrl = versionInfo.isSnapshhot ?
(angularCodeUrl + 'snapshot') :
(googleCdnUrl + (versionInfo.version || versionInfo.currentVersion));
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess if the versionInfo.version is not available then we are doing a local non-snapshot build. In which case the plunkers are going to be useless. Perhaps we should default to the snapshot though in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about setting the cdnVersion. Snapshot is also possible, but my idea was that in the rare case of no .version + testing in plnkr (basically only local), it's better to have the immediate feedback of a 404 script instead of a (probably) working app, that might not be 100% compatible with the angular version it runs on.

@petebacondarwin
Copy link
Contributor

One comment, otherwise LGTM

gkalpak
gkalpak previously requested changes Nov 4, 2016
// The currentVersion may not be available on the cdn (e.g. if built locally, or hasn't been pushed
// yet). This will lead to a 404, but this is preferable to loading a version with which the example
// might not work (possibly in subtle ways).
var examplesCdnUrl = versionInfo.isSnapshhot ?
Copy link
Member

Choose a reason for hiding this comment

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

isSnapshhot --> isSnapshot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh snap

@petebacondarwin
Copy link
Contributor

Other than the snapshhot fix it LGTM

@Narretz Narretz force-pushed the chore-docs-plnkr-version branch from 0bd2604 to a0b99f4 Compare November 4, 2016 16:47
@Narretz Narretz dismissed gkalpak’s stale review November 5, 2016 12:15

Required change has been made.

@Narretz Narretz merged commit b28f1fc into angular:master Nov 5, 2016
@gkalpak
Copy link
Member

gkalpak commented Nov 28, 2016

@Narretz , shouldn't this be backported to v1.5.x?
(If so, c625b0d should be included as well.)

@Narretz
Copy link
Contributor Author

Narretz commented Nov 28, 2016

I think I didn't backport since the change only affects the snapshot anyway. At least on angularjs.org. But we can backport for consistency

@gkalpak
Copy link
Member

gkalpak commented Nov 28, 2016

You are right, it is not necessary to backport it (theoretically). I'll backport it anyway for consistency 😁

gkalpak pushed a commit that referenced this pull request Nov 28, 2016
- docs for the snapshot will include the snapshot files from code.angularjs.org
- docs for tagged versions will include the files from the (Google) CDN
- docs for local / untagged versions will try to include the files from the (Google) CDN, which will fail. This gives immediate feedback that something is broken.

Closes #15267
Closes #15358
ellimist pushed a commit to ellimist/angular.js that referenced this pull request Mar 15, 2017
- docs for the snapshot will include the snapshot files from code.angularjs.org
- docs for tagged versions will include the files from the (Google) CDN
- docs for local / untagged versions will try to include the files from the (Google) CDN, which will fail. This gives immediate feedback that something is broken.

Closes angular#15267
Closes angular#15358
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants