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

Port 'HTTP Request' demo to Python #69

Merged
merged 3 commits into from
Feb 11, 2024

Conversation

rolandlo
Copy link
Contributor

@rolandlo rolandlo commented Feb 6, 2024

For porting the demo I followed the JavaScript implementation. Note that the Vala implementation is a bit different, because it handles exceptions. If the implementations should be as close to each other as possible this difference should be resolved eventually, I think.

Like in #62 I made all implementations use the Soup status instead of the status code to avoid magical numbers like 200.

The Vala implementation didn't work for me, so I fixed it. There seems to be an underlying issue with how Vala code is formatted automatically:

@"https://api.wikimedia.org/feed/v1/wikipedia/$language/featured/$(date.format ("%Y/%m/%d"))";

gets formatted as

@"https://api.wikimedia.org/feed/v1/wikipedia/$language/featured/$(date.format (" % Y / % m / % d "))";

which doesn't work any more (due to the added spaces). I think the Vala formatting would have to be fixed in general, but I didn't attempt to do that, so I only fixed this demo.

@lw64
Copy link
Contributor

lw64 commented Feb 6, 2024

@rolandlo can you open an issue at uncrustify about the wrong code formatting?

@rolandlo
Copy link
Contributor Author

rolandlo commented Feb 6, 2024

@rolandlo can you open an issue at uncrustify about the wrong code formatting?

OK, I will.

Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

JS 👍

Thanks for keeping everything tidy!

Copy link
Contributor

@theCapypara theCapypara left a comment

Choose a reason for hiding this comment

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

Thanks!

@theCapypara theCapypara merged commit 71ada19 into workbenchdev:main Feb 11, 2024
1 check passed
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.

4 participants