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

TODO Hunting #369

Open
jaguililla opened this issue Mar 19, 2021 · 11 comments
Open

TODO Hunting #369

jaguililla opened this issue Mar 19, 2021 · 11 comments
Labels
good first issue Earn your wings at the OSS community help wanted pinned

Comments

@jaguililla
Copy link
Member

jaguililla commented Mar 19, 2021

The code has hidden tasks, lingering among the source files... Waiting for someone to tackle them.

Would you be the brave developer to finish them?

This issue is an everlasting effort to clean the code of small tasks here and there. It won't be ever finished and stay on the 'Working' column potentially forever.

If you want to close a TODO, please reference this issue in the PR.

Most of the tasks are not long, but they don't provide much detail, feel free to ask in the comments of this issue for information.

TODO tasks are not second class citizens, bear in mind that working on them would also imply coding the proper tests (or modifying existing ones) and documenting the changes accordingly.

It's easy to discover pending tasks hidden in the code, you just have to search 'TODO', 'FIXME'... or 'WTF'.

Good luck, and ask anything you need!

@jaguililla jaguililla added good first issue Earn your wings at the OSS community help wanted labels Mar 19, 2021
@ramyaravindranath
Copy link
Contributor

Hey @jaguililla - I would like to work on few TODO tasks here.

@jaguililla
Copy link
Member Author

Thanks for your contribution! I just reviewed your PR, there were only two minor things to mention:

  1. PRs should be done to develop and will be promoted to master after that (merge on master triggers a release).
  2. A minor style issue with a space.

Good job in your first PR, I hope you find the project appealing :)

If you do, you may keep checking TODO issues as they are a good way to learn about the code base. TODOs can be big or small, clear or with missing information... You can always ask me if you need any detail.

Or if you prefer to take other tasks... be my guess, in one or two weeks I'll add new items to the backlog.

Thanks again, and happy coding!

@ramyaravindranath
Copy link
Contributor

Thanks for the feedback. I Will keep in mind to send my PRs for develop. I will keep going with TODOs for now :)

@Rishavgupta12345
Copy link
Contributor

can i work on this issue ?

@jaguililla
Copy link
Member Author

Sure! Thanks for your support :)

Please, don't make long PRs, you can make a PR per TODO solved.

I won't assign the task to you as many people can be fixing different TODOs, I will ask you to put a comment with the TODO comment you plan to take to avoid conflicts.

Thanks again... and happy coding!

@Rishavgupta12345
Copy link
Contributor

can i work on this ?

@jaguililla
Copy link
Member Author

Absolutely @Rishavgupta12345 just be sure to put a comment with the items you will be working on (for coordination sake). Thanks!

mattTea pushed a commit to mattTea/hexagon that referenced this issue Nov 6, 2021
Existing uses retain Response as type String
jaguililla added a commit that referenced this issue Nov 11, 2021
Make http_client Response body property generic - part of #369
@JordanllHarper
Copy link

I would be interested in helping on some TODO's here!

@jaguililla
Copy link
Member Author

Great!!! Just take a look at the contributing guide, and if you have any question, don't hesitate to ask. Thanks for your help :)

@dariro93
Copy link

dariro93 commented Jul 7, 2023

Hey @jaguililla . Hope to find you well.

I've been looking at some TODOs and I came across this one:

https://github.com/hexagonkt/hexagon/blob/bbd24fb1134e03a1c5feea28edc772edeada393d/http/http_client_jetty_ws/src/main/kotlin/com/hexagonkt/http/client/jetty/JettyWebSocketAdapter.kt#L24

Can you please elaborate a little bit further? I assumed you wrote (messages probably arriving here) because they both (onPing and onPong) use a ByteArray as payload and the only receiver with ByteArray payload is onWebSocketBinary, but when I executed a test invoking the ping or pong on the client, it did not reached method onWebSocketBinary... Maybe nothing has to be done here? or I'm not understanding well?

Thank you in advance.

@jaguililla
Copy link
Member Author

Hi @dariro93 , and thanks for checking the project 🙂

Being honest, I can't remember the exact details of the comment.

However, the purpose would be to handle 'onPing' and 'onPong' events by default (if I remember correctly the correct handling is specified in the standard) and allow the user to override those methods in case he/she wants to provide a specific callback.

Do this make the change easier to understand? If not, just let me know and I'll provide the details you want.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Earn your wings at the OSS community help wanted pinned
Projects
Status: Ready
Development

No branches or pull requests

5 participants