Skip to content
This repository has been archived by the owner on Aug 28, 2022. It is now read-only.

Table layout #72

Merged
merged 33 commits into from
Dec 4, 2021
Merged

Table layout #72

merged 33 commits into from
Dec 4, 2021

Conversation

ronimizy
Copy link
Contributor

Closes #9

@ronimizy ronimizy added the requirement Use cases and global aim label Nov 27, 2021
@ronimizy ronimizy self-assigned this Nov 27, 2021
@ronimizy ronimizy changed the base branch from master to dev November 27, 2021 14:39
@ronimizy ronimizy marked this pull request as draft November 28, 2021 19:29
@ronimizy ronimizy mentioned this pull request Nov 28, 2021
Open
@ronimizy ronimizy marked this pull request as ready for review November 29, 2021 10:16
annchous
annchous reviewed Nov 29, 2021
View reviewed changes
Copy link
Member

@annchous annchous left a comment

Choose a reason for hiding this comment

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

Короче, пусть дальше @hrrrrustic cмотрит

Source/SeaInk.Application/TableLayout/Models/TableModel.cs Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Models/StudentModel.cs Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Models/AssignmentModel.cs Show resolved Hide resolved
Source/SeaInk.Application/SeaInk.Application.csproj Show resolved Hide resolved
Source/SeaInk.Application/SeaInk.Application.csproj Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Components/AssignmentColumnComponent.cs Show resolved Hide resolved
@hrrrrustic
Copy link

А я тут причем вообще...

hrrrrustic
hrrrrustic reviewed Nov 29, 2021
View reviewed changes
Copy link

@hrrrrustic hrrrrustic left a comment

Choose a reason for hiding this comment

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

Я ничего не понял. ООП порнухи больше чем смысловой нагрузки кода

Source/SeaInk.Application/Exceptions/CrossedFrameException.cs Show resolved Hide resolved
Source/SeaInk.Application/Exceptions/CrossedFrameException.cs Show resolved Hide resolved
Source/SeaInk.Application/Exceptions/InvalidScaleComponentException.cs Show resolved Hide resolved
Source/SeaInk.Application/Exceptions/InvalidScaleComponentException.cs Show resolved Hide resolved
Source/SeaInk.Application/Exceptions/MissingAssignmentComponentException.cs Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/CommandInterfaces/IValueGettingLayoutComponent.cs Outdated Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Commands/AggregateValuesCommand.cs Outdated

public AggregateValuesCommand(ITableDataProvider provider)
{
_provider = provider.ThrowIfNull(nameof(provider));
Copy link

Choose a reason for hiding this comment

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

копипастни с 6 нета этот метод, чтобы не делать неймоф

Source/SeaInk.Application/TableLayout/Components/LabelComponent.cs Show resolved Hide resolved
Bibletoon
Bibletoon suggested changes Dec 1, 2021
View reviewed changes
Copy link
Member

@Bibletoon Bibletoon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Жёстко надристал полный пр TryExecute, может посмотреть в сторону FluentResult?

Source/SeaInk.Application/TableLayout/Components/PlainAssignmentColumnComponent.cs Outdated Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Indices/MergeScanningIndex.cs Show resolved Hide resolved
Source/SeaInk.Application/Exceptions/MissingStudentComponentException.cs

namespace SeaInk.Application.Exceptions
{
[Serializable]
Copy link
Member

Choose a reason for hiding this comment

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

Не резолвь этот комент пока не поменяешь кофиг сонара

Copy link
Contributor Author

@ronimizy ronimizy Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Я так понимаю, что здесь должен быть пункт Administration чтобы я смог это сделать @FrediKats

Снимок экрана 2021-12-01 в 20 57 57

Source/SeaInk.Application/TableLayout/CommandInterfaces/IReducibleLayoutComponent.cs Outdated
public interface IReducibleLayoutComponent<in TComponent>
where TComponent : LayoutComponent
{
bool TryRemoveComponent(TComponent component, IScaledTableIndex begin, ITableEditor editor);
Copy link
Member

Choose a reason for hiding this comment

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

Не очень понял как Reducible связано с Remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Expandable (расширяемый) -> Add component
Reducible (уменьшаемый) -> Remove component

Если такой нейминг кажется плохим, хоть он и корректен в плане инглиша, могу предложить поменять имя интерфейса на IShrinkableLayoutComponent

Copy link
Member

Choose a reason for hiding this comment

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

Думаю что Reducible и Shrinkable эквивалентны, так что особой разницы нет (хотя второе мне импонирует больше)
Может стоит подумать и как-то обозначить то, что убираем именно child компонент

Source/SeaInk.Application/TableLayout/Commands/AggregateValuesCommand.cs Outdated Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Commands/AggregateValuesCommand.cs Outdated Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Commands/ComponentRemoveCommand.cs Outdated Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Commands/DrawAllCommand.cs Outdated Show resolved Hide resolved
Source/SeaInk.Application/Tools/LcmCounter.cs Show resolved Hide resolved
Source/SeaInk.Application/TableLayout/Indices/ITableIndex.cs Outdated Show resolved Hide resolved
@ronimizy
Copy link
Contributor Author

ronimizy commented Dec 1, 2021

@hrrrrustic @FrediKats
#72 (comment)

Как ругается то, прям аж Major)
Снимок экрана 2021-12-01 в 21 12 21

@ronimizy
feat: added fluent results
233a761 
refactor: fallthrough commands
@hrrrrustic
Copy link

оаоаооа ммм натягивание фарш штук на сишарп
Как я и говорил еще года полтора назад в какой-то другой репе Егору - выглядит максимально уродски и непонятно зачем

Bibletoon
Bibletoon approved these changes Dec 4, 2021
View reviewed changes
Copy link
Member

@Bibletoon Bibletoon left a comment

Choose a reason for hiding this comment

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

Поправь только перед тем как мерджить

Source/SeaInk.Sample/TableLayoutSample.cs Outdated Show resolved Hide resolved
Source/SeaInk.Sample/SeaInk.Sample.csproj Outdated Show resolved Hide resolved
@ronimizy ronimizy merged commit 9ca5f2f into dev Dec 4, 2021
@ronimizy ronimizy deleted the table-layout branch December 4, 2021 07:23
@sonarcloud
Copy link

sonarcloud bot commented Dec 4, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

FrediKats pushed a commit that referenced this pull request Aug 15, 2022
@ronimizy
Merge pull request #72 from kysect/table-layout
030076c 
Table layout
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hrrrrustic hrrrrustic hrrrrustic left review comments

@annchous annchous annchous left review comments

@Bibletoon Bibletoon Bibletoon approved these changes

@FrediKats FrediKats Awaiting requested review from FrediKats

Assignees

@ronimizy ronimizy

Labels
requirement Use cases and global aim
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Реализовать хранение разметки таблицы
5 participants
@ronimizy @hrrrrustic @FrediKats @Bibletoon @annchous