I've noticed that even though you use IGitHubPluginUpdates, you still override logic for fetching release URL - based on my overview of the code, this is not needed, you follow exactly the same logic as ASF does, and there is nothing custom in your GitHub repository for that logic to not work.
Thanks to that, if the general mechanism will ever need updating, you won't have additional work to do. Overriding GetTargetReleaseURL() is intended for lower-level access where you have custom logic for finding the release - e.g. you're using GitLab or your own server instead of GitHub, but you don't, so no need to do that.
Of course your usage is correct, so you can as well disregard this notice if you want to - it's just a potential enhancement I've noticed while browsing ASFE code :+1:
chr233
commented
2 months ago
I've noticed that even though you use
IGitHubPluginUpdates, you still override logic for fetching release URL - based on my overview of the code, this is not needed, you follow exactly the same logic as ASF does, and there is nothing custom in your GitHub repository for that logic to not work.You can consider deleting whole
GetTargetReleaseURL(), and instead of that, implementGetTargetReleaseAsset(). You simply need to use this custom logic you have in that function for returning the correct asset out ofreleaseAssetsfrom the input.Thanks to that, if the general mechanism will ever need updating, you won't have additional work to do. Overriding
GetTargetReleaseURL()is intended for lower-level access where you have custom logic for finding the release - e.g. you're using GitLab or your own server instead of GitHub, but you don't, so no need to do that.Of course your usage is correct, so you can as well disregard this notice if you want to - it's just a potential enhancement I've noticed while browsing ASFE code :+1: