Skip to content
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

ref: change all func taking in jspsych to const syntax / trials to co… #529

Merged
merged 5 commits into from
Aug 14, 2024

Conversation

YUUU23
Copy link
Contributor

@YUUU23 YUUU23 commented Aug 9, 2024

After removingjsPsych passed in as params in jsPsych-global branch:

  • For procedures: change to const function syntax, call functions in original place
  • For trials: change to const jsPsych trial object syntax, move variable declarations outside the object declaration when necessary
  • javadoc: remove tags requiring jsPsych as a parameter
  • tested with running browser, dev home, dev clinic, dev clinic with video

Copy link

github-actions bot commented Aug 9, 2024

Visit the preview URL for this PR (updated for commit f310479):

https://ccv-honeycomb--pr529-ref-trial-cons-var-41ekwrl1.web.app

(expires Wed, 21 Aug 2024 21:38:22 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 4ace1dcea913a952d2a1af84b94a4421bf36e610

@YUUU23 YUUU23 self-assigned this Aug 9, 2024
@YUUU23 YUUU23 added the 4.0 Versioning: Issue in regards to version 4.0.0 release label Aug 9, 2024
@YUUU23 YUUU23 requested review from RobertGemmaJr and eldu August 9, 2024 17:08
Copy link
Contributor

@eldu eldu left a comment

Choose a reason for hiding this comment

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

Looks good! Great job Megan! A few questions and changes

src/experiment/trials/camera.js Outdated Show resolved Hide resolved
src/experiment/trials/camera.js Outdated Show resolved Hide resolved
console.error("Camera is not initialized, no data will be recorded.");
return;
}
const cameraRecorder = window.jsPsych.pluginAPI.getCameraRecorder();
Copy link
Contributor

Choose a reason for hiding this comment

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

I commented on the other out window.jsPsych. What's the order of these PRs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please let me know if this is not what you mean by order, but I think we should merge this branch into jsPsych-global and then merge jsPsych-global into feat-v4

@YUUU23 YUUU23 requested a review from eldu August 13, 2024 05:57
Copy link
Contributor

@eldu eldu left a comment

Choose a reason for hiding this comment

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

Okay! This one looks good. I did request some changes to #528 (review) which affects this PR in some ways.

So resolve those and this one looks good to go!

@YUUU23 YUUU23 merged commit 99556aa into jsPsych-global Aug 14, 2024
8 checks passed
@YUUU23 YUUU23 deleted the ref-trial-cons-var branch August 14, 2024 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.0 Versioning: Issue in regards to version 4.0.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants