dio-lab-open-source
dio-lab-open-source copied to clipboard
feat: add michelmouraresende profile
Descrição
Adiciona o profile README de michelmouraresende
Tipo de alteração
- [x] Resolução do Desafio Profile README (envie APENAS o arquivo
community/SEU_USERNAME.md
) - [ ] Correção de bug
- [ ] Nova funcionalidade
- [ ] Alteração na documentação
- [ ] Outro (especifique)
Checklist
- [x] Minhas alterações não deletam partes do projeto
- [x] Minhas alterações não introduzem novos problemas
- [x] Minha contribuição está de acordo com o Guia de Contribuição
Comentários adicionais
N/A
Maybe we can merge #3437 before? 👀 It allows for different features to be built on top like #3025 or #3640
@raphjaph @casey this ^ is ready for review now, no functionality changes, just refactoring.
I personally don't think this is a huge increase in readability, since there's just more indirection in figuring out what's going on.
I personally don't think this is a huge increase in readability.
But do you think though that it's a bit more readable?
since there's just more indirection in figuring out what's going on.
It simplify things a bit for new folks who want to read about what's going on in send.rs.
@casey Some more Context on why I went ahead with this refactor: I wanted to implement the Even Split command for rune where I can split 10k of runes into 100 each, send.rs was a big and going through that code with different branches was lots of to-and-fro. If it had been like this, understanding what's going through would be much less time consuming for new contributors and can start checking in more features like split command quickly.
I feel that even if it adds a bit more readability and simplicity, there is no harm in checking in to main branch since all unit tests are passing, unless it adds more complexity, which I don't think so.
We were a way past "Rule Of Three" and the refactoring was a due in my opinion. Currently, there are 5 sends.
To be clear: I closed this PR because I think it's a net negative and makes the code harder to read.
The rule of three is about deduplication. The PR is net +107 lines of code, and replaces a match statement which calls three different functions, with a trait with five different implementations. It is not deduplicating anything.
I think it's much worse and more complicated.
- If transfer trait has a fixed contract, bugs like this could be cought early by making sure that the postage is used for new transfer implementation - A useless postage was added to the sending runes tx? #3620
This bug was fixed with a change to create_unsigned_send_runes_transaction
. In this PR, this function was just moved from one place to another. I don't see that would have helped find that bug.
- More extensibility, for example, the most requested features in ordicord like
splitting rune
orsend to many
can have its own transfer trait implementation without bloating send.rs file.
Having six functions in a file is fine.
- send.rs is now just 52 lines of code after the refactor and super readable ❤️
It's super readable, but reading it doesn't tell you anything about what it does.
The PR is net +107 lines of code.
but does # of lines code really matter in such cases? # of lines of code would increases if a piece of code is moved to different files with new imports, new functions definitions, empty lines tends to add to net # of loc.
In this PR, this function was just moved from one place to another.
The function wasn't moved to another place as is, only just implementation was moved. The ~function~ method signature was changed to adhere to method contract create_unsigned_transaction
in the transfer
trait.
This bug (
the postage not being used for send runes
) was fixed with a change to create_unsigned_send_runes_transaction. In this PR, this function was just moved from one place to another. I don't see that would have helped find that bug.
It's true that It wouldn't have helped finding that bug, but while writing the new "send rune" implementation itself, it would have enforced the author to use postage (due to trait contract for transer
being there) In the absense of the transfer contract, there is no way to enforce/nudge that postage
need to be used. The bug was later fixed by adding postage
to create_unsigned_send_runes_transaction
function here