- Issue created by @ptmkenny
- Merge request !41When mapping, adding a new JSON source should not return help text → (Merged) created by ptmkenny
- last update
12 months ago 113 pass - Status changed to Needs review
12 months ago 1:59pm 22 April 2024 - 🇳🇱Netherlands megachriz
Having a help text would be nice, but what should it contain? Perhaps a link to the documentation of the library that is used? That would be https://github.com/SoftCreatR/JSONPath.
- Status changed to Needs work
12 months ago 2:55pm 23 April 2024 - Status changed to Needs review
12 months ago 3:51am 26 April 2024 - 🇯🇵Japan ptmkenny
I looked into adding help text, but I ran into a problem. I added a "Uses the JSONPath library" message, but then I tested JMESPath, and realized that it also uses JSONSource. So actually we would need to create separate JSONSource classes for JSONPath and JMESPath just to link to the correct library.
I can do this if you want, but I'm not sure it's worth making future code maintenance more complicated; it might be better to just remove the incorrect message.
- 🇳🇱Netherlands megachriz
Removing the help text to avoid confusion I think is better for now.
Ideally, maybe JsonSource shouldn't extend BlankSource, because it could possibly create similar issues in the future should other changes happen in BlankSource. But for that, I think there should first be another base class in Feeds that does most of what BlankSource does, except setting a description that's really only relevant for BlankSource.
- last update
11 months ago 113 pass - last update
11 months ago 113 pass -
MegaChriz →
committed e6a87a5a on 8.x-1.x authored by
ptmkenny →
Issue #3442572 by ptmkenny: Removed incorrect help text for JSON sources...
-
MegaChriz →
committed e6a87a5a on 8.x-1.x authored by
ptmkenny →
- Status changed to Fixed
11 months ago 7:48am 25 May 2024 Automatically closed - issue fixed for 2 weeks with no activity.