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

Adds support to more column types #95

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

Conversation

guilhermedelyra
Copy link

No description provided.

traduzindo p ptbr;
capitalizando tdas as palavras do titulo da issue;
add opção de criterios extras
Copy link
Owner

@andre-filho andre-filho left a comment

Choose a reason for hiding this comment

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

Thnx for the contrib, but some changes are needed. Btw, you must follow the commit rules and language to avoid having your commits squashed.

@@ -38,7 +38,7 @@ def format_description(string):
"""
Adds the header and a final new line to the issue description string.
"""
return str('**Issue description:**\n---\n\n' + string + '\n'*2)
Copy link
Owner

Choose a reason for hiding this comment

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

Keep it in english for now. Translations will be added later. Btw there is a branch named desenho-br I think, that writes the issue titles and stuff in pt-br and is deployed on ezissue-br lib

@@ -47,12 +47,12 @@ def add_prefix_to_title(title, number, prefix, subid, numerate):
"""
subid = subid.upper()
prefix = prefix.upper()
title = title.capitalize()
title = ' '.join([t.capitalize() for t in title.split(' ')])
Copy link
Owner

Choose a reason for hiding this comment

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

Split this into a function plz

@@ -82,11 +82,20 @@ def format_acc_criteria(string):
Formats the string adding the acceptance criteria header and adds a final
new line.
"""
checkboxes = add_md_checkbox(string)
acc_criteria_title = "**Acceptance criteria:**\n---\n\n"
checkboxes = add_md_checkbox(string) if string != 'Não há critérios.\n' else string
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't this logic be inside the add_md_checkbox function? It is simple, yes, but inside the function would be better organized

Formats the string adding the acceptance criteria header and adds a final
new line.
"""
checkboxes = add_md_checkbox(string) if string != 'Não há critérios.\n' else string
Copy link
Owner

Choose a reason for hiding this comment

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

Same as before

@@ -16,15 +15,15 @@
GITLAB_BASE_URL = "https://gitlab.com/api/v4"


def create_issue_json(configuration_row, values_row, repo_host):
def create_issue_json(configuration_row, values_row, repo_host, blk):
Copy link
Owner

Choose a reason for hiding this comment

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

What is blk? Use significant variable names

@@ -182,11 +187,15 @@ def main(filename, repo_host, prefix, subid, numerate, debug):
row[0] = add_prefix_to_title(
row[0], idx+1, prefix, subid, numerate)

rows = make_md_formatting(columns, rows)
columns = [x.strip() for x in columns]
Copy link
Owner

Choose a reason for hiding this comment

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

Significant variable names, plz


for row in rows:
response, issue = make_api_call(
create_issue_json(columns, row, repo_host),
create_issue_json(columns, row, repo_host, blacklist),
Copy link
Owner

Choose a reason for hiding this comment

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

If blacklist is being used on various places, should be extracted to constant

@andre-filho andre-filho changed the base branch from master to development October 10, 2020 15:57
@andre-filho andre-filho changed the title adicionando alguns suportes Adds support to more column types Oct 10, 2020
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.

2 participants