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

Линтер, тесты, сабмодули и многое другое #54

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

Conversation

SaveliyBorivets
Copy link
Collaborator

No description provided.

@SaveliyBorivets SaveliyBorivets linked an issue Jul 29, 2024 that may be closed by this pull request
@SaveliyBorivets
Copy link
Collaborator Author

Все константы сложил в constants.py
Думаю, что будет удобно, если сохранить экспорт в csv + вывод в консоль в файле с экспортом гугл(пусть весь экспорт будет в одном файле)

@SaveliyBorivets SaveliyBorivets linked an issue Jul 29, 2024 that may be closed by this pull request
@thehighestmath thehighestmath self-requested a review July 31, 2024 18:33
@thehighestmath
Copy link
Collaborator

  1. лучше тегать меня явно, хоть как-то призывать в ПР)
  2. огромные ПРы не всегда ок, но пока учимся -- местами пойдёт)
  3. буду смотреть))

@thehighestmath
Copy link
Collaborator

А где линтер?

Copy link
Collaborator

@thehighestmath thehighestmath left a comment

Choose a reason for hiding this comment

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

В целом норм
Константы увидел, рефакторинг есть

Сабмодули и лентер не нашёл (минусы большого ПРа)

Comment on lines +73 to +79
pr_data = [repository.full_name, pull.title, pull.number, pull.state, pull.base.label, pull.head.label, pull.created_at,
nvl(pull.user.name), pull.user.login, pull.user.email, '; '.join([file.filename for file in pull.get_files()]),
EMPTY_FIELD, EMPTY_FIELD, EMPTY_FIELD, EMPTY_FIELD, EMPTY_FIELD, get_info(pull.merged_by, 'name'),
get_info(pull.merged_by, 'login'), get_info(pull.merged_by, 'email'), pull.head.ref, pull.base.ref,
get_assignee_story(pull), EMPTY_FIELD if pull.issue_url is None else get_related_issues(pull.number, repository.owner, repository.name, token),
EMPTY_FIELD if pull.labels is None else ';'.join([label.name for label in pull.labels]), get_info(pull.milestone, 'title')]
info_tmp = dict(zip(PULL_REQUEST_FIELDNAMES, pr_data))
Copy link
Collaborator

Choose a reason for hiding this comment

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

не нравится так, ведь нужно помнить, что порядок важен...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

переделаю, загружу коммит с датаклассами, тегну

commit.commit.author.date, '; '.join([file.filename for file in commit.files]), commit.commit.sha, branch, commit.stats.additions, commit.stats.deletions]
info = dict(zip(COMMIT_FIELDNAMES, commit_data))
if fork_flag:
ORIG_REPO_COMMITS.append(info['commit id'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

капсом == константа, а мы в нее что-то добавляем, а потом чистим?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

получения списка репо я планировал переделать, в git_logger'е сделаю такую функцию, чтобы возвращала словарь с ключами в виде репо, а значениями в виде списка форков

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

А, это не то, в общем, исправлю)

Comment on lines +89 to +94
issue_data = [repository.full_name, issue.number, issue.title, issue.state, issue.body, issue.created_at, get_info(issue.user, 'name'),
get_info(issue.user, 'login'), get_info(issue.user, 'email'), nvl(issue.closed_at), get_info(issue.closed_by, 'name'),
get_info(issue.closed_by, 'login'), get_info(issue.closed_by, 'email'), EMPTY_FIELD, EMPTY_FIELD, EMPTY_FIELD, EMPTY_FIELD,
EMPTY_FIELD, get_assignee_story(issue), EMPTY_FIELD if issue.number is None else get_connected_pulls(issue.number, repository.owner, repository.name, token),
EMPTY_FIELD if issue.labels is None else ';'.join([label.name for label in issue.labels]), get_info(issue.milestone, 'title')]
info_tmp = dict(zip(ISSUE_FIELDNAMES, issue_data))
Copy link
Collaborator

Choose a reason for hiding this comment

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

подумать надо бы

Copy link
Collaborator

Choose a reason for hiding this comment

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

давайте попробуем посмотреть в сторону датаклассов
https://stackoverflow.com/questions/72604922/how-to-convert-python-dataclass-to-dictionary-of-string-literal
сам не пробовал, но кажется этот подход должен помочь

Copy link
Collaborator

Choose a reason for hiding this comment

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

from dataclasses import dataclass, asdict

@dataclass(kw_only=True, frozen=True)
class A:
    field1: str
    field2: str

a = A(field1='qwe', field2='asd')
print(a, asdict(a))
-->
A(field1='qwe', field2='asd') {'field1': 'qwe', 'field2': 'asd'}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Понял, сделаю так

@SaveliyBorivets
Copy link
Collaborator Author

А где линтер?

Пока разбираюсь со всем понятным. Чуточек позже будут линтеры, тесты и сабмодули)

@thehighestmath
Copy link
Collaborator

Добавить датаклассы, вместо зипа и ПР волью

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants