-
Notifications
You must be signed in to change notification settings - Fork 562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds load-from-repo functionality to gist services and new embed #1295
Conversation
CC @theacodes for research purposes and drive-by comments. |
// ID for a GitHub gist that should be loaded into the editors. | ||
String get gistId { | ||
final id = _getQueryParam('id'); | ||
return isLegalGistId(id) ? id : ''; |
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.
Out of curiosity: why an empty string instead of null
?
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.
Mostly it's a "that's how it was done in the codebase when I arrived" thing.
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.
comments are mostly nits- would like to consider using YAML instead of JSON for the metadata but otherwise LGTM
PTAL. |
Changes:
GistLoader is getting a little big at this point, but is unlikely to grow again in the short term, so I've left it as a single class with three load methods.
I've added a couple new classes to parse the JSON in the metadata file. These could have been done with built_value or json_serializeable. Since the entire file is <100 lines and neither of those packages is in use elsewhere in the project, though, I've chosen to roll my own rather than take on a new dependency and build step.
A field for "exercise mode" (dart, html, flutter) will need to be added to the Gist class. The new playground will also need to examine that field and set its editors up accordingly. This hasn't been done yet, since John is still finishing the new playground.