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

Fix memory leak when send headers in PsychicResponse #89

Merged
merged 6 commits into from
May 27, 2024

Conversation

dzungpv
Copy link
Contributor

@dzungpv dzungpv commented May 25, 2024

@hoeken
Copy link
Owner

hoeken commented May 25, 2024

Thank you very much for this PR. Everything looks good, and it's definitely a step in the right direction for using this server within ESP-IDF. As I'm not really too familiar with using ESP-IDF (which I really should learn). If it's not too much trouble, would you mind creating a simple example project in examples/esp-idf. I'm using github actions to automatically compile those and make sure that new commits don't break the existing code.

@dzungpv
Copy link
Contributor Author

dzungpv commented May 26, 2024

Yes I have finished it, but there is a problem with CI, you can try to fix it.

@hoeken
Copy link
Owner

hoeken commented May 26, 2024 via email

@dzungpv dzungpv force-pushed the master branch 3 times, most recently from 610fddb to 2915498 Compare May 27, 2024 03:20
@dzungpv
Copy link
Contributor Author

dzungpv commented May 27, 2024

It is Github CI image problem. I can build it in my machine. ESP IDF quite complicate but when you are making work, it is great.

@hoeken
Copy link
Owner

hoeken commented May 27, 2024

I'm going to merge this now. It looks like the CI stuff is failing on the LittleFS. All I really want is to make sure that the library itself compiles with esp-idf I might slim down that example to just a simple hello world page if I can't get the CI to compile. Thanks again for your work.

@hoeken hoeken merged commit ad8f71b into hoeken:master May 27, 2024
6 checks passed
@Chris--A
Copy link
Contributor

Hi @dzungpv & @hoeken,
It is highly inappropriate and disrespectful to your contributors to copy an existing commit and re submit under a different user. Regardless of attribution in the comment, my PR and commit was already there waiting for months to be entered.

@dzungpv
Copy link
Contributor Author

dzungpv commented May 31, 2024

Hi @dzungpv & @hoeken, It is highly inappropriate and disrespectful to your contributors to copy an existing commit and re submit under a different user. Regardless of attribution in the comment, my PR and commit was already there waiting for months to be entered.

I merge your code to use internal, and create the PR after some modification , hoeken can merge your commit first and merge me. I have no control to the project. @hoeken can to redo it and make Chris--A more happy?

@dzungpv
Copy link
Contributor Author

dzungpv commented May 31, 2024

@hoeken because of this PR #96, you can rebate the code, merged Chris--A code first and merge the #96

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