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

Refactoring recomendations #40

Open
wants to merge 55 commits into
base: dynamodb/operations/executor
Choose a base branch
from

Conversation

laughedelic
Copy link
Member

Hi @alberskib!
I just made some changes and left notes (sometimes questions). You can leave comments on the commit. Most of what I've done here is quite superficial refactoring. Maybe I don't get something and my changes are not suitable — just tell me.

@eparejatobes
Copy link
Member

updated this to Properties everywhere

@alberskib
Copy link
Contributor

@eparejatobes Thanks for update; Yes I even notice that you introduce some changes and publish them :) - now I have some problems because after update those libraries I have more or less 100 compilation errors (looks like problem with caches) - I will fix those problems and work on docs

@alberskib
Copy link
Contributor

Hello @eparejatobes How is going with changes in scarp, type-sets and tabula? It is ready? How is your neck? I hope you get better today.

@eparejatobes
Copy link
Member

much better thanks! I think you can start using all of them :) let me check out this and see how things are

@alberskib
Copy link
Contributor

Great.

@eparejatobes
Copy link
Member

hey @alberskib I got caught in a meeting and I just did something like what you did 👍

@eparejatobes
Copy link
Member

I'll comment on what I think can be done simpler

@alberskib
Copy link
Contributor

Great - It still needs some work but every comment that makes code better is more than welcome :)

@eparejatobes
Copy link
Member

nice @alberskib just some details :)

@alberskib
Copy link
Contributor

@eparejatobes Could you tell me how to convert item to the record?

@eparejatobes
Copy link
Member

You mean from item.Rep to record.Rep?

@alberskib
Copy link
Contributor

Exactly

@eparejatobes
Copy link
Member

IIRC item.Rep <: item.record.Rep

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.

3 participants