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

Fix issue with capabilities component #525

Merged
merged 6 commits into from
Jun 13, 2023
Merged

Fix issue with capabilities component #525

merged 6 commits into from
Jun 13, 2023

Conversation

icidasset
Copy link
Contributor

@icidasset icidasset commented Jun 12, 2023

Turns out it didn't work after all when changing the message passed to postMessage 🙈 Also got rid of the via-postmessage check which added support for the old lobby which has long been deprecated for a while, and improved the retry function which wasn't working well (actually ran the action 20 times even though initial one succeeded).

@icidasset icidasset requested a review from bgins June 12, 2023 11:49
app ? [ [ "appFolder", `${app.creator}/${app.name}` ] ] : [],
fs?.private ? fs.private.map(p => [ "privatePath", Path.toPosix(p, { absolute: true }) ]) : [],
fs?.public ? fs.public.map(p => [ "publicPath", Path.toPosix(p, { absolute: true }) ]) : [],
raw ? [ [ "raw", Base64.urlEncode(JSON.stringify(raw)) ] ] : [],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really need to add a commit hook with some default formatting 😅 I switched editors.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that particular formatting comes up fairly often. Agreed a hook would be nice for it. 💯

Copy link
Member

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Nice! Looks good. 🎉

Left one comment about naming where it looks like we went back to "webnative".

app ? [ [ "appFolder", `${app.creator}/${app.name}` ] ] : [],
fs?.private ? fs.private.map(p => [ "privatePath", Path.toPosix(p, { absolute: true }) ]) : [],
fs?.public ? fs.public.map(p => [ "publicPath", Path.toPosix(p, { absolute: true }) ]) : [],
raw ? [ [ "raw", Base64.urlEncode(JSON.stringify(raw)) ] ] : [],
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that particular formatting comes up fairly often. Agreed a hook would be nice for it. 💯

@@ -202,7 +188,7 @@ async function getClassifiedViaPostMessage(
}

const message = {
odd: "exchange-secrets",
webnative: "exchange-secrets",
Copy link
Member

Choose a reason for hiding this comment

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

Should we update the lobby to expect odd instead of webnative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll make it so that it supports both 👍

])
return new Promise((resolve, reject) => {
if (options.tries > 0) {
const unoMas = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Claro que si. Uno mas esta bien. 😄

@icidasset icidasset merged commit e8712a3 into main Jun 13, 2023
@icidasset icidasset deleted the icidasset/fix-caps branch June 13, 2023 14:50
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