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

🪲 Embedded editor broken due to missing adventure name #5361

Merged
merged 6 commits into from
Apr 3, 2024

Conversation

TiBiBa
Copy link
Collaborator

@TiBiBa TiBiBa commented Apr 3, 2024

This MR fixes an issue with the embedded editor due to a non-existing adventure object. Adding this check should fix it.

@TiBiBa TiBiBa requested a review from hasan-sh April 3, 2024 12:35
@Felienne Felienne requested a review from jpelay April 3, 2024 12:38
@Felienne
Copy link
Member

Felienne commented Apr 3, 2024

HI @TiBiBa! I guess this needs to be deployed quickly? 😬 If @jpelay approves I will deploy!

@TiBiBa
Copy link
Collaborator Author

TiBiBa commented Apr 3, 2024

HI @TiBiBa! I guess this needs to be deployed quickly? 😬 If @jpelay approves I will deploy!

Yes, if we could deploy this today that would awesome! Thanks for the quick response, I just tested the fix and it seems to solve the issue. Will prioritize the embedded view testing to prevent this from happening again in the future.

@Felienne
Copy link
Member

Felienne commented Apr 3, 2024

Deploying now! And yes adding tests will make your own future life less stressful :D

@TiBiBa
Copy link
Collaborator Author

TiBiBa commented Apr 3, 2024

Deploying now! And yes adding tests will make your own future life less stressful :D

Unfortunately it is still not working, is there someway I can verify what code is running on production? The same error still seems to exist. Edit: The last commit is 1acc76. Should this be merged first or can you deploy this PR as well?

Copy link
Member

@jpelay jpelay 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 fixing it so quickly!

Copy link
Contributor

mergify bot commented Apr 3, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit a796e40 into main Apr 3, 2024
12 checks passed
@mergify mergify bot deleted the hotfix-embedded-editor branch April 3, 2024 13:48
@jpelay
Copy link
Member

jpelay commented Apr 3, 2024

@Felienne I think we need to redeploy? Now that this is merged to main

@TiBiBa
Copy link
Collaborator Author

TiBiBa commented Apr 3, 2024

@Felienne I think we need to redeploy? Now that this is merged to main

Yes I think so too! @jpelay can you deploy or can only @Felienne do this?

@jpelay
Copy link
Member

jpelay commented Apr 3, 2024

Yes I think so too! @jpelay can you deploy or can only @Felienne do this?

No, she's the only one who can deploy.

@Felienne
Copy link
Member

Felienne commented Apr 3, 2024

Deploying now! And yes adding tests will make your own future life less stressful :D

Unfortunately it is still not working, is there someway I can verify what code is running on production? The same error still seems to exist. Edit: The last commit is 1acc76. Should this be merged first or can you deploy this PR as well?

Sorry, my bad! Your merge just missed the deploy train because the tests were still running. Now it should be there, can you check again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants