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

Load project from URL GET parameter-pradeep #98

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pradeepkumara
Copy link

@pradeepkumara pradeepkumara commented Nov 21, 2024

Description

i just changed those files where i need to change all the functions, inside IDEStartup and SplashkitOnline js

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • Documentation (update or new)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can
reproduce. Please also list any relevant details for your test configuration

Testing Checklist

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

Copy link

github-actions bot commented Nov 21, 2024

🐋 PR Preview!
Preview at https://thoth-tech.github.io/SplashkitOnline/pr-previews/98
for commit a477baf

@12345jy
Copy link
Contributor

12345jy commented Dec 4, 2024

Hi @pradeepkumara this looks great, and I believe it should implement the functionality as expected. However, there are two points that I think could be considered further:

  1. The Description could be a bit more detailed, such as including information on the testing methods used. Additionally, if you want to display a checkbox with a checkmark, you can use the following format:
    -[x] This item is completed
    -[ ] This item is not completed
  2. Regarding the message "This branch has conflicts that must be resolved", it might be related to the package-lock.json file. It might be worth pulling the latest changes and resolving any conflicts.

@12345jy
Copy link
Contributor

12345jy commented Dec 5, 2024

Overall, it looks good. By the way, one additional point to consider is that some comments seem to have been removed in this update. These comments provide valuable context and might be worth keeping in the code to ensure clarity and maintainability.

Copy link
Author

@pradeepkumara pradeepkumara left a comment

Choose a reason for hiding this comment

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

Thanks for the helpful feedback! I'll add comments in the code to explain what I've changed and what everything does.

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