Conversation
Please review my code to check if the fix is correct. I think that will covers our use cases. I am not very comfortable to have a modified version of SimplePie in the project. I know that it supports plugins, maybe we should look that way to make it easier for us if we need to update the library.
|
There are other places in FreshRSS with a similar mechanism of adding/removing credentials. It could be a good idea to use the same function(s) everywhere. For instance:
Regarding SimplePie, there are already many changes that I have done, not only the logging. Most of those changes/corrections/improvements could not have been done through plug-ins.
I am submitting pull requests to SimplePie by order of priority and likelihood of being accepted: |
There was a problem hiding this comment.
Please do not make the SimplePie library depend on a function defined in FreshRSS.
The other way around would be acceptable though.
There was a problem hiding this comment.
As you know all the changes introduced to SimplePie. Maybe it would be easier and faster if you take care of it.
There was a problem hiding this comment.
Ok, I will try to do that soon.
There was a problem hiding this comment.
Should I delete the branch or you want to reuse something I wrote?
There was a problem hiding this comment.
Just keep it for the time being. I will do some copy-paste and use it to see the line numbers.
|
Please see #815 |
Can simplify the regex (faster because anchored) for cleaning URLs based on the fact that we only have to deal with HTTP or HTTPS FreshRSS#715
Please review my code to check if the fix is correct. I think that will covers our use cases. I am not very comfortable to have a modified version of SimplePie in the project. I know that it supports plugins, maybe we should look that way to make it easier for us if we need to update the library.
See #711