Skip to content

Stam - #122

Open
kerenkay wants to merge 9 commits into
osherdp:masterfrom
kerenkay:stam
Open

Stam#122
kerenkay wants to merge 9 commits into
osherdp:masterfrom
kerenkay:stam

Conversation

@kerenkay

@kerenkaykerenkay commented Apr 1, 2020

Copy link
Copy Markdown
Collaborator

stam exercise

jhb1996
jhb1996 requested changes Apr 1, 2020
View reviewed changes

@jhb1996jhb1996 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a few small things. Other than that looks good.

Comment threadstam.py Outdated
Comment threadstam.py Outdated
Comment threadstam.py Outdated
Comment threadtest.txt Outdated
IgalKolihman
IgalKolihman requested changes Apr 2, 2020
View reviewed changes
Comment threadstam.py Outdated
Comment threadstam.py Outdated
Comment threadstam.py
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadATM.py Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadtest.txt Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadstam.py Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadstam.py Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadstam.py Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadstam.py Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadstam.py Outdated
rfire01
rfire01 reviewed Apr 4, 2020
View reviewed changes
Comment threadstam.py
jhb1996
jhb1996 reviewed Apr 5, 2020
View reviewed changes
Comment threadstam.py
@@ -0,0 +1,59 @@
"""

@jhb1996jhb1996Apr 5, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See section 3.1 for the proper way to document modules https://github.com/osherdp/python-training/blob/master/Style_Guide.md

Comment threadstam.py
newfile.write(f"keren {line.strip()} kayrich\n")


def main():

@jhb1996jhb1996Apr 5, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Follow the style guid for how to document functions (change the documentation of all of your functions to be inline with the style guide) https://github.com/osherdp/python-training/blob/master/Style_Guide.md

rfire01
rfire01 requested changes Apr 12, 2020
View reviewed changes
Comment threadstam.py
Comment on lines +1 to +2
"""The script adds firs and last name to the file's rows.
"""

@rfire01rfire01Apr 12, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fix doc

Comment threadstam.py
Comment on lines +6 to +7
PATH_TO_ORIGINAL_FILE = 1
PATH_TO_NEW_FILE = 2

@rfire01rfire01Apr 12, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  1. reorder
  2. those consts are not paths

Comment threadstam.py
Comment on lines +27 to +28
"""Adds name to the new file's rows.
"""

@rfire01rfire01Apr 12, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fix doc (one liner)

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

Reviewers

@jhb1996jhb1996jhb1996 requested changes
@rfire01rfire01rfire01 requested changes
@IgalKolihmanIgalKolihmanIgalKolihman requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kerenkay@rfire01@jhb1996@IgalKolihman