Move Lab2MetricsReporter singleton to Lab2Registry#55376
Closed
sanchitmalhotra126 wants to merge 159 commits into
Closed
Move Lab2MetricsReporter singleton to Lab2Registry#55376sanchitmalhotra126 wants to merge 159 commits into
sanchitmalhotra126 wants to merge 159 commits into
Conversation
Catch staging-next up to staging
Update staging-next
#55181) * refactor posters section * add new poster links and thumbnail images * update description copy * refactor to use a loop and make links accessible * empty commit message
fisher-alice
approved these changes
Dec 12, 2023
fisher-alice
left a comment
Contributor
There was a problem hiding this comment.
Easy-to-understand changes and cool to see how much we're already using MetricsReporter! Just left a question.
| @@ -3,7 +3,7 @@ import {SongMetadata} from '../types'; | |||
| import {commands as audioCommands} from '@cdo/apps/lib/util/audioApi'; | |||
Contributor
There was a problem hiding this comment.
Similar to LabMetricsReporter being used for both legacy and lab2, since ProgramExecutor is used by legacy and wb used by lab2, do you plan to move this file as well?
Contributor
Author
There was a problem hiding this comment.
Good point! Yeah, it would probably make sense to move it out to apps/src/dance
| try { | ||
| // Update properties for reporting as early as possible in case of errors. | ||
| Lab2MetricsReporter.updateProperties({ | ||
| Lab2Registry.getInstance().getMetricsReporter().updateProperties({ |
Contributor
There was a problem hiding this comment.
Although longer, I appreciate the clarity of this change as someone who is not as familiar with lab2.
…ge (#55392) * Fix tab navigation on lesson dropdowns for unit overview page * Add aria label * Add aria-expanded
8 tasks
Contributor
Author
|
Closing in favor of #55410 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #55339. These changes are mostly just replacement/renames; no major functionality changes:
LabMetricsReportertoLab2Registryso lab2 code can access the reporter via the Lab2Registry instanceLab2MetricsReporterwithLab2Registry.getInstance().getMetricsReporter()(note: in some class components, I wired this in through the constructor to make it easier to unit test)Lab2MetricsReporterand make the class export the default.The final follow-up to this will be to rename Lab2MetricsReporter -> LabMetricsReporter and likely move out of the
lab2directory. I wasn't sure yet where to move it so I held off on this step to avoid further inflating the PR.Warning!!
We have entered Pixel Lock for Hour of Code!
Computer Science Education Week will be happening from Dec 4 - Dec 10. Alongside this event, we will
be launching our new Hour of Code activity. Please consider any risk introduced in this PR that
could affect Dance Lab, instructions, saving and logging student progress, caching, or anything
related to the Hour of Code activities, old or new. Even small changes, such as a different button
color, are considered significant during this time. If this change will affect the new Hour of Code
activity in any way, join the morning change review to get your changes approved prior to merging.
Reach out to the Student Labs team for more details!
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: