Skip to content

Conversation

@armudgal
Copy link
Contributor

@armudgal armudgal commented Apr 14, 2019

Hi there,
As mentioned in #179.
I am not sure as manylinux2010 will be replacing manylinux1 or we still want to mention manylinux1 in the readme.

According to me:

README should majorly talk about manylinux2010 and include manylinux1 briefly
but since manylinux2010 is in demo phase, would that be a good idea?

Looking for opinions and suggestions.
Thanks

PS: Did a blunder in commit message, fixing it

@armudgal armudgal force-pushed the patch-1 branch 2 times, most recently from 96b4d42 to 4f42ffe Compare April 14, 2019 14:31
@mayeut
Copy link
Member

mayeut commented Apr 14, 2019

@armudgal, I agree that README in master should majorly talk about manylinux2010. I think a link shall be added to the manylinux1 branch in order to redirect people that want information on manylinux1 to that specific branch.

@armudgal
Copy link
Contributor Author

Sure thanks. I will make the required changes.

@trishankatdatadog
Copy link
Contributor

Speaking of which, can someone with admin superpowers (maybe @njsmith) make the manylinux1 a GitHub protected branch, so that no one can push to it directly, but need to make PRs that must be reviewed and so on?

@armudgal armudgal changed the title [WIP] Including manylinux2010 in readme Including manylinux2010 in readme Apr 16, 2019
@armudgal
Copy link
Contributor Author

One bottleneck for this PR would be to merge pypa/python-manylinux-demo#19 first, so that the demo is on the same page with this repository.

Copy link
Member

@mayeut mayeut left a comment

Choose a reason for hiding this comment

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

My previous comment still applies on the newer commit.
PEP 571 also defines the manylinux2010_i686 platform tag even though we don't have an image for this (yet).
It should read:

This was further enhanced by `PEP 571 <https://www.python.org/dev/peps/pep-0571/>`_
defining ``manylinux2010_x86_64`` and ``manylinux2010_i686`` platform tags.

@mayeut
Copy link
Member

mayeut commented Apr 27, 2019

@armudgal,
There's still a reference to PEP 513in the docker image content. Can you change that as well please ?

@mayeut mayeut self-requested a review April 27, 2019 13:36
@mayeut
Copy link
Member

mayeut commented Apr 27, 2019

@armudgal, your commit mentions Updating PEP number in Docker image content but I'm not seeing any diffs in the file for that part.

- Adding i686 tag in the readme
- Updating PEP number in Docker image content
@armudgal
Copy link
Contributor Author

I did not stage that file by mistake. Apologies 🙉

@mayeut
Copy link
Member

mayeut commented Apr 27, 2019

LGTM thanks.

@mayeut mayeut merged commit 144b628 into pypa:master Apr 27, 2019
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.

3 participants