[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
thiemowmde added a comment. I uploaded https://github.com/wmde/WikibaseDataModel/pull/491. `getWithRank` is used a single time outside of DataModel so it's not a big deal, but also not a big problem. I'm fine if somebody (@JeroenDeDauw?) doesn't want this breaking change. TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: thiemowmde Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
Bene added a comment. In https://phabricator.wikimedia.org/T98180#1282616, @thiemowmde wrote: > I would skip and stick with `getWithRank` for now. Any particular reason for this? TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Bene Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
thiemowmde added a comment. https://github.com/wmde/WikibaseDataModel/pull/490 attempts to rename `getWithPropertyId` because there are already 2 other `getByPropertyId`. I would skip and stick with `getWithRank` for now. TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: thiemowmde Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
Bene added a comment. I think breaking changes which only rename a method can be updated so easily that we can still do this if there is a major release anyways. TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Bene Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
Bene added a comment. To clarify, this would mean `getByIdentifier` but `removeWithIdentifier` or also `removeByIdentifier`? Should we also rename things like `getWithRank` to `getByRank` or `hasStatementWithGuid` to `hasStatementByGuid`? TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Bene Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
thiemowmde added a comment. I created https://github.com/wmde/WikibaseDataModel/issues/486 to make this visible in the planned milestone 3.0 at https://github.com/wmde/WikibaseDataModel/milestones/3.0. My argument: The GUID is an identifier. Selecting stuff "with" an identifier sounds odd, it should be selected "by" it's identifier. TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: thiemowmde Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
thiemowmde added a comment. > I think this is obvious Not at all. A "StatementWithGuidMap" sounds wrong, I agree on that. But I really don't see what's wrong with "get by id", especially if the method is guaranteed to return a single element, in contrast to the "get with property id" and "get with rank" methods that return lists. So still, why did we renamed this? What was wrong with it? TASK DETAIL https://phabricator.wikimedia.org/T98180 EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: thiemowmde Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
Bene added a comment. Counting hints in deprecation notices is not fair, is it? My stats (using grep) tell me: - "ByGuid" is used only once in a method name, `StatementByGuidMap::getStatementByGuid`. This should be renamed as well to `StatementByGuidMap::getStatementWithGuid` - Other occurences always are the name of an array or class which does indexing by Guid. In this case using "by" is correct. - "WithGuid" is used 7 times in method names, 3 times in deprecated `Claims`, 2 times in `StatementByGuidMap` and 2 times in `StatementList` - All those methods clearly identify or handle a single statement so in those cases using "with" is correct. (For reference, I used `grep ByGuid src/ -r` and `grep WithGuid src/ -r`) Some replies: > I would like to see an external reference for the "grouping vs. selecting" > statement. I think this is obvious but I can search for one. > I still don't see a problem with "consistency in this [the `StatementList`] > class". There is no other occurrence of "ByGuid" or "WithGuid". However, there are `getWithPropertyId` and `getWithRank`. Furthermore, in `StatementByGuidMap` we have `hasStatementWithGuid` and `removeStatementWithGuid`. > I picked the name for consistency with > `StatementByGuidMap::getStatementByGuid.` I'd propose to change that name as well. See above. TASK DETAIL https://phabricator.wikimedia.org/T98180 REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign . EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Bene Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
thiemowmde added a comment. - I would like to see an external reference for the "grouping vs. selecting" statement. - I still don't see a problem with "consistency in this [the `StatementList`] class". There is no other occurrence of "ByGuid" or "WithGuid". - I picked the name for consistency with `StatementByGuidMap::getStatementByGuid`. - "WithGuid" occurs 5 times in DataModel code, 3 times in the deprecated `Claims` class we are phasing out right now. - "ByGuid" occurs 9 times in code and comments. TASK DETAIL https://phabricator.wikimedia.org/T98180 REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign . EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: thiemowmde Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
[Wikidata-bugs] [Maniphest] [Commented On] T98180: Fix inconsistent With vs. By naming in DataModel
Bene added a comment. There is an obvious difference between both use cases. The StatementByGuidMap does grouping by guid while the method returns a single statement with guid. Using by for grouping and with for selecting is just the correct use of English prepositions. TASK DETAIL https://phabricator.wikimedia.org/T98180 REPLY HANDLER ACTIONS Reply to comment or attach files, or !close, !claim, !unsubscribe or !assign . EMAIL PREFERENCES https://phabricator.wikimedia.org/settings/panel/emailpreferences/ To: Bene Cc: JeroenDeDauw, Bene, thiemowmde, Aklapper, Wikidata-bugs, aude ___ Wikidata-bugs mailing list Wikidata-bugs@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs
