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

Refactoring: Rename dotpath into dotnotation #6369

Merged
merged 13 commits into from
Apr 25, 2024

Conversation

math-GH
Copy link
Contributor

@math-GH math-GH commented Apr 21, 2024

Follow up of #6317

renaming from dot path to dot notation

(for a better review: go through each commit. I tried to bundle it in each commit)

It will affect exported OPMLs that were already exported and hat dotpath information. But it was published as dev version, so I would say that it is okay to rename the attribute.

@math-GH math-GH added the System care Everything related to the system care label Apr 21, 2024
@math-GH math-GH added this to the 1.24.0 milestone Apr 21, 2024
@@ -161,8 +161,8 @@ private function createFeed(array $feed_elt, FreshRSS_Category $category, bool $
case strtolower(FreshRSS_Export_Service::TYPE_XML_XPATH):
$feed->_kind(FreshRSS_Feed::KIND_XML_XPATH);
break;
case strtolower(FreshRSS_Export_Service::TYPE_JSON_DOTPATH):
$feed->_kind(FreshRSS_Feed::KIND_JSON_DOTPATH);
case strtolower(FreshRSS_Export_Service::TYPE_JSON_DOTNOTATION):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be sufficient to add another case line here to support OPML exports with the previous name

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in cf17221
Test welcome

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not so a big fan of having legacy code because of changed feature within a dev release, but it is ok.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested to work.
edge is our rolling release, and there are likely many users of the JSON feature already, so avoiding breaking changes is always good. I have myself this feature deployed on multiple instances already.

@Alkarex Alkarex merged commit 154a367 into FreshRSS:edge Apr 25, 2024
2 checks passed
@math-GH math-GH deleted the rename-dotpath-into-dotnotation branch April 25, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System care Everything related to the system care
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants