Skip to content

Crystax support for Feedparser #1324

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

Closed
wants to merge 2 commits into from
Closed

Crystax support for Feedparser #1324

wants to merge 2 commits into from

Conversation

kuzeyron
Copy link
Contributor

@kuzeyron kuzeyron commented Aug 9, 2018

In this PR I've added support for Feedparser to work with python3crystax.

@AndreMiras
Copy link
Member

Hi @kuzeyron thanks for the pull request. Could you please fix a couple of issues:

  • tox is failing, simply run tox from the root path and it will point you to the errors
  • if this pull request is just about changing this line: depends = [('hostpython2', 'python3crystax'), 'setuptools'] please keep it noise-free by not moving line around and avoiding changing space by tabs. It might just be an editor issue.
  • last, I think this could easily be just one single commit rather than two. Basically you need to squash them. If you don't know how to do it all, give us write access to your pull request and we can do it for you.

@kuzeyron
Copy link
Contributor Author

@AndreMiras I edited that line in the webbrowser. Then I made a pull request through git. But I can't see it anywhere. I don't think I changed the whitespace on the file. Only directly added the crystax part.

@AndreMiras
Copy link
Member

OK I see, it's a good opportunity for you to learn a bit more about git/GitHub and the shell.
Basically if you used the online editor, what it does is it creates a fork. So you can simply clone it:

git clone git@github.com:kuzeyron/python-for-android.git

And then work on it, the workflow should be something like:

cd python-for-android/
git reset --soft HEAD~1
vim pythonforandroid/recipes/feedparser/__init__.py
tox
git add pythonforandroid/recipes/feedparser/__init__.py
git commit --amend

Basically the git reset --soft HEAD~1 will squash your two commits and create only one. Then you edit with your favorite editor (vim obviously). Then you run the tox test to make sure you didn't introduce any regression. Then you cheat the commit log with git add && git commit --amend.
Feel free to Google the commands you didn't understand to find out more.

@KeyWeeUsr
Copy link
Contributor

KeyWeeUsr commented Aug 14, 2018

@kuzeyron @AndreMiras https://github.com/servo/servo/wiki/Beginner's-guide-to-rebasing-and-squashing let git do the work :) also, git's documentation is quite nice nowadays.

@AndreMiras
Copy link
Member

@kuzeyron ping 😄
Do you still have interest with this PR? I think it's a useful one, but we jus want to keep the source control clean. If you struggle with rebasing and squashing feel free to create just a new PR with just that simple change.

@kuzeyron
Copy link
Contributor Author

Hey André! I haven't had any interests in trying to re-PR the Feedparser.
I'm doing other projects so my time is being spent on them instead.
Feel free to fix that part!

@AndreMiras
Copy link
Member

Thanks @kuzeyron for the feedback.
I've created the follow up pull request here #1376

@AndreMiras AndreMiras closed this Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants