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.

Sign up for GitHub

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

Jump to bottom

Added XML response to web.py #26

Closed
wants to merge 5 commits into from
Closed

Added XML response to web.py #26

wants to merge 5 commits into from

Conversation

ba11b0y
Copy link
Contributor

@ba11b0y ba11b0y commented Oct 5, 2017

No description provided.

@ba11b0y
Copy link
Contributor Author

ba11b0y commented Oct 5, 2017

Fixes #18.

shubhodeep9
shubhodeep9 requested changes Oct 5, 2017
View reviewed changes
Copy link
Member

@shubhodeep9 shubhodeep9 left a comment

Choose a reason for hiding this comment

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

The code is oddly indented,
Function scrape has a tabular indentation, where as request has spacial indentation.

@ba11b0y
Copy link
Contributor Author

ba11b0y commented Oct 5, 2017 •
edited
Loading

I have converted tabs to spaces using the online edit option but when I view the changes online, it still shows 8 tabs.
😕

@ashwini0529
Copy link
Member

Hey @invinciblycool See if this helps you? http://paste.ubuntu.com/25679718/

Also, in from xmljson import badgerfish as bf , I don't think xmljson is a default library.
You might need to update the requirements file.

ashwini0529
ashwini0529 reviewed Oct 5, 2017
View reviewed changes
Copy link
Member

@ashwini0529 ashwini0529 left a comment

Choose a reason for hiding this comment

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

Hey @invinciblycool All looks good from my side.
It would be great if you could update the usage documentation. 😄

hackr/web.py Outdated
from xml.etree.ElementTree import tostring

import requests
from xmljson import badgerfish as bf
Copy link
Member

Choose a reason for hiding this comment

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

xmljson needs to be mentioned in the requirements file. Can you please update it?

@ba11b0y ba11b0y closed this Oct 5, 2017
@ba11b0y ba11b0y deleted the add/xmltoweb branch October 5, 2017 15:24
@ba11b0y
Copy link
Contributor Author

ba11b0y commented Oct 5, 2017 •
edited
Loading

Sorry but I had messed up my remote branch.
Really sorry for the mess.
Opened a new PR at #27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashwini0529 ashwini0529 ashwini0529 left review comments

@shubhodeep9 shubhodeep9 shubhodeep9 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ba11b0y @ashwini0529 @shubhodeep9