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

Convert all .py files to be valid Python #98

Merged
merged 1 commit into from
Mar 7, 2018

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Aug 15, 2017

raise NotImplementedError('Please replace this line with your code...') signals to the user where they need to put their code.

This approach enables the codebase to be tested with lint tools like flake8, pyflakes, pylint, etc.

@cclauss
Copy link
Contributor Author

cclauss commented Nov 20, 2017

Bump on this PR?

1 similar comment
@cclauss
Copy link
Contributor Author

cclauss commented Feb 7, 2018

Bump on this PR?

@donnemartin
Copy link
Owner

Hi @cclauss, many of the code samples are intentionally not fully runnable...some are pseudocode or stubbed out. I feel raising NotImplementedError and asking to have them implemented goes beyond that scope.

I'd prefer to use pass instead of an exception. What do you think?

@cclauss
Copy link
Contributor Author

cclauss commented Feb 24, 2018

Is this OK or do you want the comment changed or removed altogether?

Copy link
Owner

@donnemartin donnemartin left a comment

Choose a reason for hiding this comment

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

Could we remove the "# Please replace this line with your code..." comments?

@cclauss
Copy link
Contributor Author

cclauss commented Feb 27, 2018

Done.

@cclauss
Copy link
Contributor Author

cclauss commented Mar 3, 2018

Bump?

Copy link
Owner

@donnemartin donnemartin left a comment

Choose a reason for hiding this comment

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

Looking good! Almost there, just one small change.

def remove_user(self, user): # ...
def add_user(self, user):
pass
def remove_user(self, user):
Copy link
Owner

Choose a reason for hiding this comment

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

Could we add a line break here?

`raise NotImplementedError('Please replace this line with your code...')` signals to the user where they need to put their code.

This approach enables the codebase to be tested with lint tools like flake8, pyflakes, pylint, etc.
@cclauss
Copy link
Contributor Author

cclauss commented Mar 4, 2018

Done.

@donnemartin donnemartin merged commit f099a0a into donnemartin:master Mar 7, 2018
@donnemartin
Copy link
Owner

@cclauss thank you!

@cclauss cclauss deleted the patch-2 branch March 7, 2018 02:37
Md-Shahnawaz added a commit to Md-Shahnawaz/system-design-primer that referenced this pull request Mar 11, 2018
* zh-TW: Add proper line break to links (donnemartin#139)

* Add Greek translation link (donnemartin#140)

* Convert all .py files to be valid Python (donnemartin#98)

* zh-Hans: Fix typos (donnemartin#141)

* Add Spanish translation link (donnemartin#142)
godelian added a commit to godelian/system-design-primer that referenced this pull request Oct 9, 2019
* Add license disclaimer (donnemartin#76)

Clearly denote the repository license is from me, not my employer (Facebook).

* Fix asynchronism link in Mint README (donnemartin#69)

* Fix lowercase typo in Layer 7 load balancing title (donnemartin#74)

* Fix donnemartin#79: Inaccuracy in Twitter timeline and search example (donnemartin#81)

* Fix typo (donnemartin#75)

* Init spot_size member in parking lot exercise (donnemartin#83)

* Add prev to Node class (donnemartin#82)

For managing an LRU cache, we need to use a doubly linked list for move_to_front() operation.

* Fix reducer argument in pastebin exercise (donnemartin#85)

* Add polish translation link (donnemartin#89)

* Fix typo in denormalization section (donnemartin#95)

* Add policy for adding new blogs to Company engineering blogs section (donnemartin#96)

* Add Russian translation link (donnemartin#91)

* Treat solution subdirectories as containing Python packages (donnemartin#99)

* Add Japanese translation link (donnemartin#101)

* Add traditional Chinese translation link (donnemartin#105)

* Add Italian translation link (donnemartin#106)

* Fix typo: considering to consider in scaling AWS solution (donnemartin#109)

* Add API rate limiter design link (donnemartin#108)

* Add Korean translation link (donnemartin#111)

* Simplify logic for is_ace and is_face_card in deck of cards solution (donnemartin#113)

* Add Persian translation link (donnemartin#115)

* Correct Redis list structure in Twitter solution (donnemartin#116)

* Add Japanese Translation (donnemartin#114)

* Update Japanese translation link (donnemartin#118)

* ja: Fix typo (donnemartin#119)

* Make some minor wording/formatting changes (donnemartin#120)

* add Twitter Handles 3,000 Images Per Second

* Add Netflix in Company Architectures (donnemartin#122)

* Add Vietnamese translation link (donnemartin#128)

* Add Scaling Uber in Company Architecture (donnemartin#123)

* Update master-slave section anchor (donnemartin#129)

* Add Facebook Live Streams (donnemartin#125)

* Update contributing guidelines - PR squash (donnemartin#135)

* Update re:Invent url (donnemartin#137)

* Add Traditional Chinese translation (donnemartin#133)

* zh-TW: Add proper line break to links (donnemartin#139)

* Add Greek translation link (donnemartin#140)

* Convert all .py files to be valid Python (donnemartin#98)

* zh-Hans: Fix typos (donnemartin#141)

* Add Spanish translation link (donnemartin#142)

* Add missing colons to class methods (donnemartin#143)

* Add missing self variables to Deck of Cards solution (donnemartin#145)

* Add deque import to Social Graph solution (donnemartin#147)

* Change LARGE to VehicleSize.LARGE in parking lot solution (donnemartin#146)

* zh-TW: Fix comment format (donnemartin#155)

* Fix coding errors (donnemartin#149)

* Fix dict KeyError (donnemartin#152)

* Fix dict KeyError (donnemartin#153)

* Adding missing self variable (donnemartin#158)

* Add missing enum imports (donnemartin#157)

* Change variable seller to category in Mint solution (donnemartin#159)

* zh-Hans: Fix typo (donnemartin#161)

* Replace broken SQL tuning links (donnemartin#163)

* zh-Hans: Fix PostgreSQL typo (donnemartin#166)

* Fix donnemartin#148: Add State(Enum) to social_graph_snippets.py (donnemartin#167)

* ja: Swap VARCHAR and CHAR translation error (donnemartin#169)

* Add Arabic translation link (donnemartin#177)

* Add newlines - PEP8 style (donnemartin#173)

* zh-TW: Improve translations (donnemartin#176)

* Fix typo in LRU cache solution (donnemartin#182)

* Fix broken links (donnemartin#180)

* Add German translation link (donnemartin#188)

* Add Thai translation link (donnemartin#190)

* Fix broken URL (donnemartin#194)

* Fix error in Twitter timeline solution (donnemartin#196)

* Fix wording in call center solution (donnemartin#197)

* Resolve donnemartin#195: Fix broken GitHub URLs (donnemartin#199)

* zh-Hans: Fix typo: 'Twitter' (donnemartin#200)

* Update Twitter/Facebook exercise description (donnemartin#202)

* Fix typo in CAP theorem section (donnemartin#205)

* Fix broken links (donnemartin#204)

* Fix donnemartin#126: Update link to Anki decks (donnemartin#206)

* Fix typo in Design Pastebin.com exercise (donnemartin#210)

* zh-Hans: Fix translation (donnemartin#208)

* zh-Hans: Update translation (donnemartin#212)

* zh-Hans: Update translation (donnemartin#209)

* Update Cassandra architecture links (donnemartin#213)

* Update README intro (donnemartin#216)

* Resolve donnemartin#214: Add reference links to message queues section (donnemartin#218)

* Update Scalability for Dummies link (donnemartin#224)

* ja: Fix typo (donnemartin#226)

* zh-TW: Update index anchors (donnemartin#227)

* Add Link: A 360 Degree View Of The Entire Netflix Stack (donnemartin#229)

* Fix donnemartin#228: Address mutex latency discrepancy (donnemartin#233)

* Add Bengali translation link (donnemartin#242)

* Add missing word in cache write through discussion (donnemartin#245)

* zh-Hans: Fix typo (donnemartin#246)

* Fix grammar in document store section (donnemartin#247)

* Fix typo in Twitter timeline and search solution (donnemartin#251)

* Update document-store to document store (donnemartin#255)

* Enable Python syntax highlighting in Pastebin sample code (donnemartin#257)

* Fix broken SQL link in Scaling AWS exercise (donnemartin#258)

Fix broken SQL link in Scaling AWS exercise

* Update HDFS design link to the latest version (donnemartin#275)

* Enable syntax highlighting in all python code snippets (donnemartin#268)

* Translate language list (donnemartin#252)

* Add Ebook generation script (donnemartin#207)

* Add availability in numbers section (donnemartin#237)

* Update language lists in translations (donnemartin#280)

* Add Hebrew translation link (donnemartin#286)

* zh-Hans: Translate Pastebin solution (donnemartin#273)

* ja: Fix typo of Big-O notation in KVS section (donnemartin#292)

* JA: Fix mistranslation in Horizontal scaling section

- The Japanese translation is ambiguous about “vertical scaling” means scaling out or scaling up.
- The word “expensive” is missing in the Japanese translation.

* JA: Fix mistranslation in Reverse proxy (web server) section

- Fix mistranslation of parallel structure. (not information/blacklist/limit, but hide/blacklist/limit)

* ja: Fix mistranslation in "Horizontal scaling"

* JA: Fix mistranslation in Weak consistency section (donnemartin#299)

* JA: Fix mistranslation in Push CDNs section (donnemartin#300)

* JA: Fix mistranslation in Federation section (donnemartin#303)

* ja: Fix translation in "Anki flashcards" (donnemartin#306)

* ja: Fix translation in “Disadvantage(s): load balancer” (donnemartin#307)

* ja: Fix translation in Service Discovery section (donnemartin#308)
Godfrey-Ndungu pushed a commit to Godfrey-Ndungu/system-design-primer that referenced this pull request Jun 20, 2023
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.

2 participants