Feedback #1

github-classroom · 2023-03-20T16:43:23Z

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.
In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
Click the Files changed or Commits tab to see all of the changes pushed to main since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

Click the Files changed tab to see all of the changes pushed to main since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
Click the Commits tab to see the commits pushed to main. Click a commit to see specific changes.
If you turned on autograding, then click the Checks tab to see the results.
This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.
For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @wokuparalyzed @RoketFlame @Lesh79

…trees and nodes

fix: fix text in LICENSE.txt

feat: release simple_add function

feat: add fun compareTo for BSNode

refactor: rename simple_add to simpleAdd

feat: add fun equals for KeyValue and BSNode

fix: change returned type of simpleContains to NodeType fix: field parent is correct now (in simpleAdd)

refactor: reformat source files

…ting pull request into main or release

struct: change struct of project for gradle building

fix: change main class in build.gradle.kts struct: delete App.kt (useless)

Signed-off-by: RoketFlame <[email protected]>

refactor: format Neo4j repo Signed-off-by: RoketFlame <[email protected]>

…ntu (CI)

…tories) and app (GUI)

rudolf101

В целом, неплохо, но есть что исправить

rudolf101 · 2023-05-02T14:43:16Z

CONTRIBUTING.md

@@ -0,0 +1,40 @@
+# Внесение правок


Славно иметь так называемый Welcome note, где вы поблагодарите человека за потенциальный вклад в проект, а также расскажете чем именно он может помочь, например:

Завести баг

Предложить новую фичу

Исправить существующий баг

etc.

rudolf101 · 2023-05-02T14:50:05Z

CONTRIBUTING.md

+# Внесение правок
+
+## Основные советы
+


Не говорите что НЕ нужно делать, лучше предоставьте примерный пайплайн человеку, который хочет контрибьютить. Можно сказать, что вы используете Github Flow, поэтому все изменения принимаются через ПР'ы, расскажите о возможных требованиях к названию ветки из мастера. Если новый код требует тестов, нужно ли добавить тесты, если изменилось API, нужно ли обновить документацию? Попросите человека проверить, что его код удовлетворяет принятому в проекте codestyle и приложите ссылку, если таковой имеется.

rudolf101 · 2023-05-02T14:52:33Z

CONTRIBUTING.md

+1. Не используйте merge, только rebase (для сохранения линейной истории коммитов)
+2. Не менять чужие ветки без крайней необходимости
+3. Перепроверьте историю коммитов перед созданием пулл реквеста
+4. **Перепроверьте, что вы в правильной ветке**, никогда не коммитьте напрямую в main


История коммитов отлично сохраняются и при использовании merge, вероятно, тут нужен контекст. Остальные пункты немного похожи на воду.

rudolf101 · 2023-05-02T14:55:00Z

CONTRIBUTING.md

+
+## Правила для пулл реквестов
+
+**НЕ ТЫКАТЬ НА ЗЕЛЕНУЮ КНОПОЧКУ `REBASE AND MERGE` БЕЗ РЕВЬЮ**


Почему вы все запрещаете? Расскажите лучше о том, как сделать ПР правильно, как сделать заголовок, какое должно быть содержание, какие устанавливать лейблы. Что за разговорный стиль, вы же пишите документ для потенциальных контрибьютеров, а тут "тыкать", "сливать"...

rudolf101 · 2023-05-02T14:59:49Z

CONTRIBUTING.md

+
+Если тыкаете на зеленую кнопочку, то **убедитесь**, что на ней написано `REBASE AND MERGE`
+
+Ревью происходит в виде комметариев к пулл реквестам, обсуждения в чате команды и личном общении.


Расскажите кого именно из команды нужно указывать в качестве ревьювера, вместо рассказа о том как проходит ревью, скажите, например, что вы постараетесь качественно заревьювить новый код, но это может занять время.
Зачастую идея ревью как раз в публичности, поэтому "обсуждения в чате команды и личном общении" звучит довольно интересно.

rudolf101 · 2023-05-02T15:28:12Z

DOCS.md

+
+## Storing Trees
+
+`teemEight` provides ~~`JsonRepository`, `SqlRepository` and~~ `Neo4jRepository` to save & load trees.


В ридми написано, что .json и sql поддерживается, тут они перечеркнуты?

rudolf101 · 2023-05-02T15:28:56Z

lib/src/main/kotlin/repository/Neo4jRepository.kt

+    private val sessionFactory = SessionFactory(configuration, "repository")
+    private val session = sessionFactory.openSession()
+
+    //converts Neo4jNodeEntity to SerializableNode.


Комменты

rudolf101 · 2023-05-02T15:33:53Z

lib/src/main/kotlin/repository/Repository.kt

+    protected val strategy: Serialization<T, NodeType, TreeType, *>
+) {
+    //An extension function that converts an instance of a NodeType to a serializable
+    //representation of a SerializableNode using the serialization strategy


В общем, советую изучить KDoc syntax если вы так любите писать комментарии.

rudolf101 · 2023-05-02T15:49:55Z

lib/src/test/kotlin/trees/RBTreeTest.kt

+            )
+        }
+    }
+}


Возможно хочется увидеть для RBTree тесты с крайними случаями, например, когда удаляем ноду, которая приходилась правым сыном для красного родителя и так далее.

rudolf101 · 2023-05-02T15:52:24Z

lib/src/test/kotlin/trees/BSTreeTest.kt

+    }
+
+    @Test
+    fun `check invariant while deleting`() {


Общее ко всем тестам, вместо проверки всего и сразу лучше добавить больше проверок на специфичные случаи, например на удаление несуществующей ноды, или на удаление корня, и тд.

Было бы славно еще добавить отчет о покрытии.

fix: value in KeyValue must be not null test: update test for keyValue

rudolf101

Косметические моменты поправить, а так следующие наблюдения:

Приложение оформлено довольно хорошо
Возле кнопки "выбрать файл" хотелось бы увидеть кнопку "создать пустой", без ридми непонятно что пустой файл нужно создать самому и выбрать после
Кнопка перехода на домашнюю страницу должна учитывать были ли сохранены изменения, а то случайно жмакнул туда и потерял весь прогресс. Обычный пайплайн в такой ситуации следующий: файл не изменился с последнего сохранения -> переходим сразу домой, если файл изменился, делаем уведомление с подтверждением, что все несохраненные изменения будут утеряны -> переходим домой / сохраняем файл и только потом идем домой
В целом, кнопки подобраны хорошо, нативно понятно что они делают
Все функции, зума, перемещения, изменения раскладки и тд работают хорошо, немного смущает, что вся левая часть приложения под окном ввода не прозрачная
Кажется не работает кнопка поиска, что бы я туда не вводил ничего не происходит
Круто было бы подкрутить логи
Кнопка для сброса лейаута была бы кстати, раз вы все равно его сбрасываете при добавлении ноды
Хочется видеть какие-то уведомления когда что-то происходит, условно я добавляю вершинку, появляется уведомление, что вершина добавлена. Если попробую добавить такую же, то появится уведомление, что вершина уже существует, и так со всем
По нажатию на вершину или при наведении хочется видеть значение вершины
Раз сейчас поддерживается только .json, зачем остальные пункты в меню?) Можно их пока убрать, чтобы не смущали. Забавно, что это информация только в ридми. Как вариант сделайте выбор по умолчанию на .json, а остальные варианты оставьте невыбираемыми с пометкой "Coming soon" или что-то в этом роде

Итого, мне понравилось, видно, что немного не хватило времени/желания, с другой стороны патч в доте сам себя не изучит.

rudolf101 · 2023-05-03T14:55:47Z

app/src/main/kotlin/Main.kt

+//    val testTree = avlRepo.loadByName("test")
+//    println(testTree.preOrder()) // output pre-order traversal of tree
+//    avlRepo.save("wow", tree)
+//    println(avlRepo.getNames())


Почему это закомменчено? Это должно быть здесь?

rudolf101 · 2023-05-03T15:05:01Z

DOCS.md

+avlRepo.save("test", tree)
+val testTree = avlRepo.loadByName("test")
+println(testTree.preOrder()) // output pre-order traversal of tree
+```


В доках обязательно нужно добавить раздел с приложением.

rudolf101 · 2023-05-03T15:05:58Z

app/src/main/kotlin/Main.kt

+import java.awt.Dimension
+
+fun main() = application {
+    Window(


Очень не хватило примеров, например .json деревьев где-нибудь в папке sample, чтобы человек мог быстренько скачать, посмотреть как работает.

github-classroom bot and others added 30 commits March 20, 2023 16:43

Setting up GitHub Classroom Feedback

0de1819

first commit, add .gitignore, README.md, init Gradle

3cbfea0

feat: add Deployment paragraph in README.md, add basic structure for …

c008b4a

…trees and nodes

feat: add CONTRIBUTING.md

117df44

fix: fix text in LICENSE.txt

fix: fix generics in interface Tree

f7a83f8

feat: release simple_add function

fix: fix interface Node

d5751e5

feat: add fun compareTo for BSNode

fix: add .idea files (trash)

f17f3b8

feat: change generics in Tree, now it has more clear structure

fe3d134

fix: change interface Tree, root in ABSTree is protected

6898fb5

refactor: rename simple_add to simpleAdd

fix: fix compareTo for Node and BSNode

1ca3445

feat: add fun equals for KeyValue and BSNode

feat: add early BSTree

c7ee8b5

fix: delete .idea

86a611e

fix: add .idea to .gitignore

3749a5f

feat: add fun get for BSTree

caa7bb2

feat: add filed parent for interface Node

064af09

fix: fix version of gradle

c5c003b

feat: add funs simpleDelete, getMinimal and getMaximal

0a84450

fix: change returned type of simpleContains to NodeType fix: field parent is correct now (in simpleAdd)

fix: change contains function to be compatible with simpleContains

b13cc6d

feat: add copyright header for source files

3a81f3d

refactor: reformat source files

feat: add workflows for github, it has to check project build on crea…

79de548

…ting pull request into main or release

fix: change platform for build and tests

e6f0ee6

struct: change struct project for gradle building

542a092

feat: init correct build project

a299c66

feat: add build directory

740e220

feat: init correct gradle project

c0f3561

struct: change struct of project for gradle building

fix: refactor fun get, now it uses simpleContains from ABSTree

c135272

fix: change main class in build.gradle.kts struct: delete App.kt (useless)

fix: fix funs getMinimal nad getMaximal

b4e7919

fix: huge refactoring of methods, now it are more simple and clear

5a945fd

fix: minor changes in the use of ABSTree functions

ab537d2

feat: add conversion KeyValue to string

c5f45a3

RoketFlame and others added 12 commits April 24, 2023 05:06

feat: add way to store trees with .json file

83a6aeb

feat: add a way to store trees in SQL database

bca7503

Signed-off-by: RoketFlame <[email protected]>

feat: add field typeOfTree in JsonRepository

8d216c7

Signed-off-by: RoketFlame <[email protected]>

fix: fix AVLNode

4f019e6

refactor: format Neo4j repo Signed-off-by: RoketFlame <[email protected]>

feat: implement getNames for Neo4jRepo

86ff2c0

feat: implement very early visualizer for trees

297ad58

feat: update version of gradle, add building project on macOS and ubu…

05f8fa6

…ntu (CI)

feat: update properties for gradle, prepare for restructure of project

445c8fa

struct: huge refactor, now project include lib (with trees and reposi…

1c3d41a

…tories) and app (GUI)

feat: add comments to code to repository part

934d808

feat: add comments to code to trees package

2b6eb36

fix: update CI script

17c8217

RoketFlame force-pushed the main branch from c956531 to 17c8217 Compare April 29, 2023 11:11

hotfix: update README

ead2d74

rudolf101 suggested changes May 2, 2023

View reviewed changes

RoketFlame added 9 commits May 2, 2023 23:56

feat: simplify work with JsonRepository, update dependencies

f25ab8a

fix: value in KeyValue must be not null test: update test for keyValue

feat: first version of App, so dirty code (sorry)

868ae7a

feat: first version of App, so dirty code (sorry)

32e3c97

feat: simplify some cast expressions

2cb077d

feat: add some checks for correct opening tree

9672f88

feat: add a button to return to the home screen

97f923f

fix: fix name of text fields

c99838f

feat: update CONTRIBUTING.md, DOCS.md, LICENSE and README.md

f276c79

feat: update README.md, add tips for using viewer

8d4c82d

rudolf101 approved these changes May 3, 2023

View reviewed changes

RoketFlame force-pushed the main branch from a0398bb to 96e024d Compare May 3, 2023 17:03

fix: fix bug with empty key field

cf27697

RoketFlame force-pushed the main branch from 96e024d to cf27697 Compare May 3, 2023 17:14

fix: Change licence of stolen files

0b7e482

RoketFlame force-pushed the main branch from a0f9e70 to 0b7e482 Compare May 5, 2023 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feedback #1

Feedback #1

Uh oh!

github-classroom bot commented Mar 20, 2023 •

edited

Loading

Uh oh!

rudolf101 left a comment

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 May 2, 2023

Uh oh!

rudolf101 left a comment

Uh oh!

rudolf101 May 3, 2023

Uh oh!

rudolf101 May 3, 2023

Uh oh!

rudolf101 May 3, 2023

Uh oh!

Uh oh!

		# Внесение правок

		## Основные советы


		## Правила для пулл реквестов

		НЕ ТЫКАТЬ НА ЗЕЛЕНУЮ КНОПОЧКУ `REBASE AND MERGE` БЕЗ РЕВЬЮ


		Если тыкаете на зеленую кнопочку, то убедитесь, что на ней написано `REBASE AND MERGE`

		Ревью происходит в виде комметариев к пулл реквестам, обсуждения в чате команды и личном общении. No newline at end of file


		## Storing Trees

		`teemEight` provides ~~`JsonRepository`, `SqlRepository` and~~ `Neo4jRepository` to save & load trees.

+                          )
+                      }
+                  }
+              }

                
                    No newline at end of file

Feedback #1

Are you sure you want to change the base?

Feedback #1

Uh oh!

Conversation

github-classroom bot commented Mar 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rudolf101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rudolf101 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-classroom bot commented Mar 20, 2023 •

edited

Loading