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

Use IPC names for DIP packages #73

Closed
wants to merge 3 commits into from
Closed

Conversation

ouabache
Copy link
Contributor

bumped version 0.1 -> 0.1.1
changed create date to 2020 1/18 12:00:00
UUIDs not affected

bumped version 0.1 -> 0.1.1
changed create date to 2020 1/18  12:00:00
UUIDs not affected
@dbrgn
Copy link
Member

dbrgn commented Jan 18, 2020

Thanks! We follow PEP8 to keep the code style consistent. For checking, we use flake8, you can install it (e.g. via pip install flake8 and simply run the flake8 command in the root directory.

There are a few failures reported by the tool. Could you fix them?

Here are the failures, mostly about horizontal alignment, which PEP8 recommends against:

./generate_dip.py:14:8: E221 multiple spaces before operator
./generate_dip.py:15:11: E221 multiple spaces before operator
./generate_dip.py:16:15: E221 multiple spaces before operator
./generate_dip.py:17:16: E221 multiple spaces before operator
./generate_dip.py:18:18: E221 multiple spaces before operator
./generate_dip.py:20:11: E221 multiple spaces before operator
./generate_dip.py:21:17: E221 multiple spaces before operator
./generate_dip.py:21:25: W291 trailing whitespace
./generate_dip.py:25:1: E303 too many blank lines (3)
./generate_dip.py:109:1: W293 blank line contains whitespace
./generate_dip.py:112:14: E222 multiple spaces after operator
./generate_dip.py:112:30: E225 missing whitespace around operator
./generate_dip.py:113:10: E221 multiple spaces before operator
./generate_dip.py:113:14: E222 multiple spaces after operator
./generate_dip.py:114:10: E221 multiple spaces before operator
./generate_dip.py:114:14: E222 multiple spaces after operator
./generate_dip.py:114:23: E226 missing whitespace around arithmetic operator
./generate_dip.py:115:10: E221 multiple spaces before operator
./generate_dip.py:115:14: E222 multiple spaces after operator
./generate_dip.py:115:26: E226 missing whitespace around arithmetic operator
./generate_dip.py:115:56: E225 missing whitespace around operator
./generate_dip.py:116:10: E221 multiple spaces before operator
./generate_dip.py:116:14: E222 multiple spaces after operator
./generate_dip.py:117:10: E221 multiple spaces before operator
./generate_dip.py:117:14: E222 multiple spaces after operator
./generate_dip.py:119:1: W293 blank line contains whitespace
./generate_dip.py:120:9: E303 too many blank lines (2)
./generate_dip.py:122:86: E231 missing whitespace after ','
./generate_dip.py:122:88: E231 missing whitespace after ','
./generate_dip.py:122:90: E231 missing whitespace after ','
./generate_dip.py:122:92: E231 missing whitespace after ','
./generate_dip.py:122:94: E231 missing whitespace after ','

@dbrgn dbrgn self-assigned this Jan 18, 2020
Copy link
Member

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Added a few change suggestions.

generate_dip.py Outdated Show resolved Hide resolved
generate_dip.py Outdated Show resolved Hide resolved
generate_dip.py Outdated Show resolved Hide resolved
generate_dip.py Outdated Show resolved Hide resolved
@dbrgn dbrgn mentioned this pull request Jan 18, 2020
@dbrgn dbrgn changed the title Changed names to IPC-7251 Use IPC names for DIP packages Jan 18, 2020
@ouabache
Copy link
Contributor Author

ouabache commented Jan 18, 2020 via email

@dbrgn
Copy link
Member

dbrgn commented Jan 19, 2020

Thanks! There are still some warnings generated by flake8. I'll fix them myself.

I noticed that the way we calculate the top offset (using a fixed offset independent from the pin count) is actually wrong. I will fix that, and will integrate your name changes into the PR with those fixes.

@ouabache
Copy link
Contributor Author

ouabache commented Jan 19, 2020 via email

@ouabache
Copy link
Contributor Author

ouabache commented Jan 19, 2020 via email

@dbrgn
Copy link
Member

dbrgn commented Jan 26, 2020

Closing in favor of #75. @ouabache I included your changes in the other PR.

@dbrgn dbrgn closed this Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants