-
Notifications
You must be signed in to change notification settings - Fork 0
02-VCS #2
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
base: master
Are you sure you want to change the base?
02-VCS #2
Conversation
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- было бы круто, если бы status ещё и выводил текущий бранч
- разумно, хотя и не обязательно, автоматически переключаться на новый бранч сразу при его создании (вряд ли часто создают бранч просто чтобы он был)
- про конфликт нелишне бы писать, какой файл приводит к конфликту
- на трависе тесты не проходят, надо поправить
Насколько я понимаю, на момент дедлайна (23.03) значительная часть функциональности не была написана, так что максимум баллов за эту задачу ограничен.
| throws DirectoryExpected, IOException, InvalidDataInStorage, | ||
| FileFromWorkingDirectoryExpected, RegularFileExpected { | ||
| if (args.length == 0) { | ||
| System.out.println("Arguments expected"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь лучше какой-нибудь хелп выводить
| @@ -0,0 +1,4 @@ | |||
| package ru.spbau.bachelor2015.veselov.hw02.exceptions; | |||
|
|
|||
| public class AlreadyExists extends Exception { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Исключения принято именовать с суффиксом Exception (например, обсуждение http://softwareengineering.stackexchange.com/questions/187944/the-suffix-exception-on-exceptions-in-java)
| * @param path a path to check. | ||
| * @return true if path lies inside repository inner storage, false otherwise. | ||
| */ | ||
| public boolean isInsideStorage(final @NotNull Path path) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А они точно должны быть public? Не то чтобы часто используемые клиентами операции, при желании их можно реализовать самим, и это целых три метода в публичном API, который, как известно, чем меньше и аккуратнее, тем лучше
| } | ||
|
|
||
| /** | ||
| * Build a nem commit from a content of an index file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nem -- new
| return builder.toString(); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Собственно, про такой мегарепозиторий я говорил на паре --- он пытается делать вообще всё, поэтому почти 1300 строк кода. Имеет смысл его как-то зарефакторить (например, хотя бы вынести внутренние классы и постараться перенести в них больше логики).
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
По поводу продвинутой функциональности --- всё работает, причём так, как ожидалось. Единственное, что надо доделать --- это вкрутить логирование, оно было нужно по условию.
No description provided.