Skip to content

Refactor VideoModal to ES-module initializer; use object mapping#2176

Open
aman-arabzadeh wants to merge 1 commit intojakartaee:srcfrom
aman-arabzadeh:refactor-video-modal
Open

Refactor VideoModal to ES-module initializer; use object mapping#2176
aman-arabzadeh wants to merge 1 commit intojakartaee:srcfrom
aman-arabzadeh:refactor-video-modal

Conversation

@aman-arabzadeh
Copy link
Copy Markdown

No description provided.

@netlify
Copy link
Copy Markdown

netlify bot commented May 28, 2025

Deploy Preview for jakartaee ready!

Name Link
🔨 Latest commit 942c301
🔍 Latest deploy log https://app.netlify.com/projects/jakartaee/deploys/68370e954f21e80008295a59
😎 Deploy Preview https://deploy-preview-2176--jakartaee.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@oliviergoulet5
Copy link
Copy Markdown
Member

Thanks for submitting the PR @aman-arabzadeh!

A few things to note:

  1. You will need to create an Eclipse account and sign the Eclipse Contributor Agreement before we can merge this into the codebase. You can see that there is a status check for this, and it is currently failing.
  2. This code is no longer running. The VideoModal function is no longer being invoked in your patch. It used to be an IIFE.
  3. You swapped the strings from single quotes to double quotes. Our codebase uses single quotes.

Aside from those three points, the refactor looks good! We appreciate your support.

@aman-arabzadeh
Copy link
Copy Markdown
Author

aman-arabzadeh commented May 28, 2025 via email

…clude args

Signed-off-by: Koray Aman Arabzadeh <aman.arabzadeh98@gmail.com>
@aman-arabzadeh aman-arabzadeh force-pushed the refactor-video-modal branch from e2d8e27 to 942c301 Compare May 28, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants