Skip to content

Check wit 1#2

Open
RuthMahallaMilano wants to merge 75 commits into
mainfrom
check-wit-1
Open

Check wit 1#2
RuthMahallaMilano wants to merge 75 commits into
mainfrom
check-wit-1

Conversation

@RuthMahallaMilano

@RuthMahallaMilano RuthMahallaMilano commented Feb 28, 2022

Copy link
Copy Markdown
Owner

Hi,
Please check the project folder.
Thanks!

Comment thread project/add.py
def copy_file_to_staging_area(
staging_area_path: Path, path: Path, relative_path: Path
) -> None:
parents_path = relative_path.parents[0]

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.

האם בהכרח יש איבר 0? אם כן, אולי שווה להצהיר על זה שורה לפני: assert relative_path.parents או אפילו באופן מובהק יותר: assert len(relative_path.parents) > 0

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

ממה שאני מבינה זה לפי ההגדרה של PurePath.parents - האיבר ה-0 נותן את כל האבות של הקובץ. אם אין- זה נותן פשוט ".", אז זה עדיין קיים בעצם. ואז השאלה אם יש parents_path.name (שורה אח"כ).
האם עדיין יש טעם ל- assert?

Comment thread project/branch.py Outdated
Comment thread project/checkout.py Outdated
Comment thread project/checkout.py
Comment thread project/utils.py Outdated
Comment thread project/graph.py Outdated
Comment thread project/merge.py Outdated
Comment thread project/merge.py Outdated
Comment thread project/status.py Outdated
Comment thread project/status.py Outdated
RuthMahallaMilano and others added 2 commits April 12, 2022 23:11
Co-authored-by: Yam Mesicka <yammesicka@gmail.com>
Co-authored-by: Yam Mesicka <yammesicka@gmail.com>
Comment thread project/status.py Outdated
if not glob(str(path_in_repository)):
raise NotImplementedError(
f"The file {file_path} was deleted. Deleting files not implemented yet."
)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

החלטתי להוריד את השגיאה הזאת מstatus, כי תכלס אם מוחקים קובץ זה לא אמור להפריע לבדיקת הסטטוס של הקבצים, ולעומת זאת אם אשאיר את השגיאה הזאת היא כל פעם תקפוץ בגלל שהקובץ נמחק.
עדיין יש אותה בmerge.

@RuthMahallaMilano

Copy link
Copy Markdown
Owner Author

Hi Yam, I believe I finished. I left some questions\ comments. Thanks a lot :)

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