-
Notifications
You must be signed in to change notification settings - Fork 27.4k
chore(docs-gen): create plnkr examples with the correct version #15358
Conversation
79eb5e4
to
0bd2604
Compare
// might not work (possibly in subtle ways). | ||
var examplesCdnUrl = versionInfo.isSnapshhot ? | ||
(angularCodeUrl + 'snapshot') : | ||
(googleCdnUrl + (versionInfo.version || versionInfo.currentVersion)); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
One comment, otherwise LGTM |
// 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 ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isSnapshhot --> isSnapshot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh snap
Other than the |
0bd2604
to
a0b99f4
Compare
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 |
You are right, it is not necessary to backport it (theoretically). I'll backport it anyway for consistency 😁 |
- 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
- 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
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?