Remove all _ from function names
#29
Comments
|
It is also a standard that the noun part is singular. From the examples given in Get-TwitterFavorites_List
Get-TwitterFollowers_Ids
Get-TwitterFollowers_List
Get-TwitterFriends_Ids
Get-TwitterFriends_List
Get-TwitterFriendships_Incoming
Get-TwitterFriendships_Lookup
Get-TwitterFriendships_NoRetweetsIds
Get-TwitterFriendships_Outgoing
Get-TwitterFriendships_ShowI think it would look better if the names were split on |
|
I'm inviting everyone to voice their opinion on this. I like the current function name structure. But I can easily change all the function names. What do you think? |
The issue with making them singular, is that they wouldn't match the API endpoint. |
|
Imo, it doesn't matter what the API endpoint is. The whole point of writing PS to wrap them is that you abstract their API away into something that's familiar to PS users. I think @nohwnd is right in his suggestion to move to singular nouns and to have the post-underscore part moved into switches for one function rather than a bunch of functions. |
I like the idea, having less functions, and more dynamic parameters. but then, I might as well wrap everything into one function, and have a parameter for |
Not a big fan of making the module 'different' that the API. The idea is to facilitate the interaction with the API, not invent a whole new naming strategy for each endpoint. |
|
The idea is to abstract the api implementation away from the user so they don't need to know that the endpoint is called a specific thing. So having singular commands for each endpoint and then parameters to deal with list or ids is the way I'd handle it. Under the hood they might all call your invoke-twitterapi command but the user doesn't need to see/use that. |
|
I like the idea of exposing less functions. And having grouped functions with more dynamic parameters. The functions are currently generated dynamically from the Twitter API Documentation, using the scripts in the APIHelper folder. If anyone wants to 'engineer' a solution to do it all dynamically, you're more than welcome to make a PR for it. We could always keep the ~125 helper functions, put them in /private/ and add these dynamic helper functions in the /public/ section? Anyone else has any suggestions? If having |
|
@mkellerman it may be worthwhile for you to share this script that is dynamically creating these because as far as I was aware Twitter haven't implemented Swagger/OpenAPI documentation. (Actually I just seen the comment above about the API helper folder) This also raises the concern that if Twitter change their API (which they have been know to do and break things) then if you are building form a json file where and how is this file generated? |
|
Using what @nohwnd had in the example this Get-TwitterFavorites_List
Get-TwitterFollowers_Ids
Get-TwitterFollowers_List
Get-TwitterFriends_Ids
Get-TwitterFriends_List
Get-TwitterFriendships_Incoming
Get-TwitterFriendships_Lookup
Get-TwitterFriendships_NoRetweetsIds
Get-TwitterFriendships_Outgoing
Get-TwitterFriendships_Showshould become Get-TwitterFavorites
Get-TwitterFavorites -List
Get-TwitterFollowers
Get-TwitterFollowers -Id
Get-TwitterFollowers -List
Get-TwitterFriend
Get-TwitterFriend -Id
Get-TwitterFriend -List
Get-TwitterFriendship
Get-TwitterFriendship -Incoming
Get-TwitterFriendship -Lookup
Get-TwitterFriendship -NoRetweetsIds
Get-TwitterFriendship -Outgoing |
https://github.com/mkellerman/PSTwitterAPI/tree/master/APIHelper
If Twitter changes their API, we are SOL one way or another. the JSON file is produced by my APIHelper. |
|
@kilasuit @nohwnd @thomasrayner @ChrisLGardner @mkellerman and I would like to invite you four to an AzureDevOps project for a possible redesign of this module. Let us know if you're interested and we can send you an invite. This will be the first collaboration project that either of us has worked on, so if you have any suggestions that would be fantastic. |
|
I'm happy to help out with that. Feel free to send the invite to whatever email I have attached to github, it's probably got an MSDN linked to it so you should be able to use that license option.
…________________________________
From: Dave Carroll <notifications@github.com>
Sent: Saturday, March 9, 2019 2:18:59 PM
To: mkellerman/PSTwitterAPI
Cc: Chris Gardner; Mention
Subject: Re: [mkellerman/PSTwitterAPI] Remove all `_` from function names (#29)
@kilasuit<https://github.com/kilasuit> @nohwnd<https://github.com/nohwnd> @thomasrayner<https://github.com/thomasrayner> @ChrisLGardner<https://github.com/ChrisLGardner>
@mkellerman<https://github.com/mkellerman> and I would like to invite you four to an AzureDevOps project for a possible redesign of this module. Let us know if you're interested and we can send you an invite.
This will be the first collaboration project that either of us has worked on, so if you have any suggestions that would be fantastic.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#29 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AQDqFte_LkfPb9MRKwD-cf6VkPpnfvv1ks5vU8LTgaJpZM4agLcq>.
|
|
Is there a need to have it away from GH and not just be a fork here? |
|
That was my suggestion :p |
|
I tried creating a project on here but couldn't link it to a forked repo. Honestly, I was wanting to learn Azure DevOps and thought it would be a good place to start. A project board could provide a place for discussion without having to create commits to a repo. |
|
It’s a temporary fork, that will either get merged into the main repo, or get canned (or stay it’s own project). |
|
Issues work fine for most discussions. I'd figured Azure DevOps would be for the build and release side rather than project management.
…________________________________
From: Marc R Kellerman <notifications@github.com>
Sent: Saturday, March 9, 2019 5:24:07 PM
To: mkellerman/PSTwitterAPI
Cc: Chris Gardner; Mention
Subject: Re: [mkellerman/PSTwitterAPI] Remove all `_` from function names (#29)
It’s a temporary fork, that will either get merged into the main repo, or get canned (or stay it’s own project).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#29 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AQDqFptrjXDJfVxxuB_yLN3DBYzAEM03ks5vU-43gaJpZM4agLcq>.
|
|
AzureDevOps is already used on the main repo for the CI/CD build/test/publish process. |
|
I can’t fork my own project lol |
|
forked to https://github.com/PowerShellModules/PSTwitterAPI if you are all on the PS Slack lets chat over there & get you invited into the GH org |
|
There is a #PSTwitterAPI channel in discord. Not sure if it’s replicated in Slack |
It is :-) |
|
I came here to express the exact same thoughts. Is there any update on this (other than the forked repo)? |
|
No update on this. I think for now, i might just remove the '_' all together, instead of trying to redesign all the functions with dynamic switches. Unless someone wants to step up and help design it differently. |
|
I'm actually happy to get involved here. It's been on my backlog to look for a decent Twitter module for a while and I came across yours which does everything I need. It's not just now ... PowerShelly, as has been said 😄 For me I think leaving the original function as is, for backwards compatibility, makes some sense. Unless somebody has another suggestion. But also putting together functions based on functionality rather than slaving them to the API 'names' makes a lot of sense too. It's something I'd like to take the time to look at and submit some PR's. I'm assuming they would be submitted here rather than the original repo? |
|
@pauby, Also, a Slack channel was created for this module as well. Are you on the Slack PowerShell workspace? |
|
@thedavecarroll You should be able to add me to that in the Settings of the repo. I am on PowerShell Slack. You can find me as |
|
Anyone has an update on this item? Did someone come up with a better solution? |
|
I keep scheduling time to work on this and get lost in something else. That's not reporting progress but it is reporting that I've not forgotten about this! |
|
Looking at it surely the code you use to autogen in here just needs a small amendment PSTwitterAPI/APIHelper/New-APIHelperFunction.ps1 Lines 9 to 13 in 953070c possibly only needing line 11 changed to the below $ResourceName = $ResourceNames[0] + ($ResourceNames[1..999] -Join "" -Replace ":", "")But this is a guess from a quick scan and not testing the code so I could be wrong |
|
Though perphas the auto gen creates private functions and then work is done to create better user friendly functions like as per this comment #29 (comment) In time you could even auto gen those commands using a lookup table but that seems like overkill. |
|
Okay, so keeping it simple.. Get-TwitterStatuses_Lookup becomes Get-TwitterStatusesLookup I’m okay with this change. This is definitely a break change to anyone who’s using this module current. Should I keep the old name as an alias? |
|
My understanding, like @kilasuit mentioned, is that this all needs rewritten. We can use the code that is there but the function names need to reflect what they do rather than Twitter's API. My 2p - |
|
But this is a module to interact with an API.. why wouldn’t the function names represent the APIs? We are better off making a new module: PSTwitter, and have a limited, user-friendly, functions that uses PSTwitterAPI as a dependency. So everyone is happy lol |
Function names should reflect the task that it performs or the results it brings back. You don't interact with an API for the sake of interacting with an API. You interact with it do get results or perform a task. Function names should reflect that.
I completely agree. There is nothing that we can do with this particular module that would not break everything for everybody (unless we keep all of the old functions but then the module name would not reflect the new functions, if that makes sense). |
|
Can I suggest a new module name? (When I said 'consensus' I did remember reading a thread on this somewhere - please don't ask me where and if you know where it was, or disagree please say so 😅 ) |
|
Do we care about breaking changes? The module is versioned 0.x so anything goes at that point, preferably avoiding breaking changes but sometimes they happen and people should be pinning versions or reviewing releases before the update (I'm well aware that's not how most people work but they need to learn to do it). |
|
My thoughts around a new module were mainly aimed at the name. I don't think it fits with a module in the way I described it. But I agree with absolutely everything you said 😄 |
|
I’m sorry to force the issue guys.. but I personally need to interact with the API in all its shapes and forms. Due to hundreds of API endpoints, and different ways someone might interact with these endpoints, this module was created to expose all these endpoints in a quick and easy way for a powershell user. Like I said, let’s create a ‘Twitter’ module, that imports this PSTwitterAPI, and provides a user meaningful name for the few functions most users use/need. But the point of this module is to interact with the API. |
|
Being able to script the creation of functions is a great idea, but you must check the source for garbage data. For instance, just looking at Send-TwitterDirectMessages_IndicateTyping, I see a few minor issues.
Has The function Get-TwitterGeo_Search -attribute:street_address '1212 My Street'
errors : {@{code=6; message=No data available for the given coordinate.}}
query : @{url=https://api.twitter.com/1.1/geo/search.json?attribute:street_address=street_address&lat=1212%20My%20Street; type=search; params=}
Get-TwitterGeo_Search -'attribute:street_address' '1212 My Street' | fl
errors : {@{code=12; message=You must provide valid coordinates, IP address, query, or attributes.}}
query : @{url=https://api.twitter.com/1.1/geo/search.json?lat=-attribute%3Astreet_address&long=1212%20My%20Street; type=search; params=}As suggested, a completely new module, whether dependent upon this one or not, would be needed to provide all PowerShell users, from beginners to highly skilled developers, a consistent experience with current best practices. Personally, I would like to see a PowerShell module for Twitter that would provide or answer the following.
If anyone would like to greenfield a new community repo (with the goal of creating a new module) and establish some guidelines around the list above or other aspects that I didn't cover, I would be glad to assist. Thanks, |
|
Everything you said is right. I haven’t been able or the need to test out all the automated functions my API helper provided. It’s only there to setup the framework for most of the functions. I agree that user focused powershell functions should be provided to the end user. So maybe we simply make the API endpoint functions private, and the new functions public? Is there a way to import-module with a switch to expose ‘all’ the endpoints when we import the module? So we don’t have to maintain two separate modules? |
You previously said this:
If you want to use the current module functions then making them private in a new module is not going to work for end users as they won't be able to access them. My thoughts are, as @thedavecarroll said along with others, that we create a new module (Let's just call it On this comment:
I am happy to create that but I think there is likely to be a better place for it in a 'community repo' (again as @thedavecarroll said). |
|
If we can make the endpoint functions private/public with a switch when we load the module, this might make things simpler than having 2 separate modules. Even if it would be an environment variable we need to set prior to loading the module. This would only be used for a select few users.. as the user friendly functions would give 99% of what the general public needs. So i vote the rewrite PSTwitterAPI.. keeping the endpoint functions (but without the '_'), and having a new set of user friendly functions that uses the endpoint functions as the backbone. |
|
My 2 cents:
|
I agree with @vexx32 this would be a little weird to do. I don't know any other module that does this and I'm not actually sure of a practical way to do it (disclosure, I've not looked or had any experience doing it as I don't know any other module that does it, as I said).
I think putting in all the work for a select few users is counter productive. For those users, this module works for them as it stands. We've only had 3 votes on that last comment (it was only posted yesterday though). However, does anybody have a'community repo' in mind? Failing that I'm going to create it and we can shift it later? |
|
We can repurpose this one if this is how the community wants it. I’m eager to see how this is going to evolve. I can add collaborators. Who’s in? |
|
I would personally prefer to see it under a community repo rather than a personal one if multiple people are going to work on it. |
End user usability - you can always use module scope calling to call the private functions directly like so (pulled from my PesterHelpers module at https://github.com/PowerShellModules/PesterHelpers/blob/master/Public/Export/Export-AllModuleFunction.ps1 - take note of the line $AllFunctions = & $moduleData {Param($modulename) (Get-command -CommandType Function -Module $modulename).Where{$_.CommandType -ne 'Cmdlet'}} $module $AllFunctions = & $moduleData {Param($modulename) (Get-command -CommandType Function -Module $modulename).Where{$_.CommandType -ne 'Cmdlet'}} $module
$PrivateFunctions = Compare-Object $PublicFunctions -DifferenceObject $AllFunctions -PassThru -Verbose:$VerbosePreference
Foreach ($PrivateFunction in $PrivateFunctions){
Write-Verbose "We Found $($PrivateFunction.Name) that is not being Exported"
}
$PublicFunctions | ForEach-Object { Export-Function -Function $_.Name -ResolvedFunction $_ -OutPath $OutPath -Verbose:$VerbosePreference }
$PrivateFunctions | ForEach-Object { Export-Function -Function $_.Name -ResolvedFunction $_ -OutPath $OutPath -PrivateFunction -Verbose:$VerbosePreference }
2 places to update = more likelihood one lags behind & PSTwitter already exists
As seen in this thread I'd see that isn't the case
I said the same months (if not over a year ago) and hence it was forked to https://github.com/powershellmodules/PSTwitterAPI/ (which I own and can grant users access to) |
|
Listen guys.. I admire what you guys want to do. I think re-writing the module from scratch, as a PS user friendly focus, is fantastic. It's just been a year, and nothing has been done so far by anyone. We are improving this module, and we aren't working on another one. I'm going to close this item. Those of you who want to work on a fresh new module, please continue the conversation on https://github.com/powershellmodules/PSTwitterAPI/ I will continue to update and improve this module in the mean time, as this project will continue to be used by some. |
Been there 😁 And agree with all that you posted above. |
It is a standard practice that no
_or-(other than betweenVerb-Noun) are used in function namesTherefore this issue is to raise about removing them to fall inline with standard PowerShell Practices
The text was updated successfully, but these errors were encountered: