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

modularize the script #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

sanjana091001
Copy link
Contributor

Signed-off-by: Sanjana Nayar sanjananayar2k1@gmail.com

Signed-off-by: Sanjana Nayar <sanjananayar2k1@gmail.com>
reparo.py Show resolved Hide resolved
reparo.py Outdated
PATH_TO_FILE2 = items2.replace(dirname, '')
PATH_TO_FILE = PATH_TO_FILE2.strip('/')
CHAOSS_HTTP = "https://api.github.com/repos/chaoss/"
LINK = "https://api.github.com/repos/chaoss/"
Copy link
Owner

Choose a reason for hiding this comment

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

seems like this is a constant variable, how about putting this on the top?

reparo.py Outdated Show resolved Hide resolved
@vchrombie
Copy link
Owner

Hi @sanjana091001
I have left some comments, please check and solve them.

Let me know if you have any doubts.
Also, push commits to this branch. After this PR becomes perfect, you can merge all the commits into a single commit.

Thanks for the effort. :)

Comment on lines 31 to 32
years = "2015-2020"
owner = "Bitergia"
Copy link
Owner

Choose a reason for hiding this comment

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

these are constants too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sanjana091001
I have left some comments, please check and solve them.

Let me know if you have any doubts.
Also, push commits to this branch. After this PR becomes perfect, you can merge all the commits into a single commit.

Thanks for the effort. :)

yup!
I'll make the changes

Signed-off-by: Sanjana Nayar <sanjananayar2k1@gmail.com>
Comment on lines +106 to 122
with open(file_name, 'r') as f:
contents = f.readlines()
i = 0
for item in contents:
if item.startswith('#'):
i += 1

with open(items2, 'w') as f:
with open(file_name, 'w') as f:
f.writelines(contents[i:])

with open(items2, 'r') as f:
with open(file_name, 'r') as f:
contents = f.readlines()
contents.insert(0, result+"\n")

with open(items2, 'w') as f:
with open(file_name, 'w') as f:
contents = "".join(contents)
f.write(contents)
Copy link
Owner

Choose a reason for hiding this comment

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

How about having the whole thing in one seperate function?

Copy link
Owner

Choose a reason for hiding this comment

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

seems like all these steps are being called in a for loop
https://github.com/vchrombie/reparo/pull/7/files#r399426460

we just need to replace the license only once, and that too in the last step

Comment on lines +67 to +68
for item in FILES:
api_func(item, dirname)
Copy link
Owner

Choose a reason for hiding this comment

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

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.

None yet

2 participants