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.

Sign up for GitHub

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

Jump to bottom

FIXES ISSUE #5637 (IDFGH-3712) #5638

Closed
wants to merge 1 commit into from
Closed

FIXES ISSUE #5637 (IDFGH-3712) #5638

wants to merge 1 commit into from

Conversation

DaividFrank
Copy link

If I somehow missed any logs please let me know and I'll be happy to fix them.

@CLAassistant
Copy link

CLAassistant commented Jul 24, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title FIXES ISSUE #5637 FIXES ISSUE #5637 (IDFGH-3712) Jul 24, 2020
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

@Alvin1Zhang Alvin1Zhang requested a review from ginkgm July 27, 2020 01:06
@projectgus
Copy link
Contributor

projectgus commented Jul 29, 2020 •
edited
Loading

Noting this in relation to #5637 (to create the cross-link).

EDIT: Sorry I missed this was in the title, mentioning it there didn't create a cross-link for some reason.

@DaividFrank After discussing this internally, we may fix it in another way as we have a separate configuration setting for disabling logs already. Instead we'll make sure no instance of __FILE__ is incorrectly passed to the logging mechanism (we have the log tag for exactly this reason).

The PR & the linked issue will be updated once this is fixed. Thanks again.

@DaividFrank
Copy link
Author

@projectgus I'll be more than happy to do it if possible =]

@projectgus
Copy link
Contributor

@DaividFrank Thanks for the offer but as it's a small change I think we can make it internally as well, and then merge it. Thanks for drawing this to our attention in the first place!

projectgus pushed a commit that referenced this pull request Aug 20, 2020
@Ethan-CYQ
driver, http_client, web_socket, tcp_transport: remove __FILE__ from …
caaf62b 
…log messages

__FILE__ macro in the error messages adds full paths to the production binarys, remove __FILE__ from the ESP_LOGE.

Closes #5637
Merges #5638
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution, changes have been merged with 37cd624. Thanks.

@jgcjoec jgcjoec mentioned this pull request Oct 20, 2020
Closed
gabsuren pushed a commit to gabsuren/esp-protocols-1 that referenced this pull request Apr 8, 2022
@Ethan-CYQ @suren-gabrielyan-espressif
driver, http_client, web_socket, tcp_transport: remove __FILE__ from …
82ab41e 
…log messages

__FILE__ macro in the error messages adds full paths to the production binarys, remove __FILE__ from the ESP_LOGE.

Closes espressif/esp-idf#5637
Merges espressif/esp-idf#5638


* Original commit: espressif/esp-idf@caaf62b
euripedesrocha pushed a commit to euripedesrocha/esp-protocols that referenced this pull request Oct 17, 2022
@Ethan-CYQ @euripedesrocha
driver, http_client, web_socket, tcp_transport: remove __FILE__ from …
bdd4357 
…log messages

__FILE__ macro in the error messages adds full paths to the production binarys, remove __FILE__ from the ESP_LOGE.

Closes espressif/esp-idf#5637
Merges espressif/esp-idf#5638


* Original commit: espressif/esp-idf@caaf62b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ginkgm ginkgm Awaiting requested review from ginkgm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DaividFrank @CLAassistant @Alvin1Zhang @projectgus