search

VFPX / Thor

Tool manager for FoxPro
39 stars 19 forks source link

Enabled flexible use of URLs #228

Closed lscheffler closed 9 months ago

lscheffler commented 9 months ago

Solving #225.

See CONTRIBUTING what files are touched and how to use.

Download https://github.com/lscheffler/Thor/raw/dev/ThorUpdater/Thor.zip to create a test installation running from the dev branch of the lscheffler fork.

Update:
It is intended that there is only a little number of modules in the dev branch. This should not harm any other branch.

Jimrnelson commented 9 months ago

@lscheffler

I have cancelled this pull request for the following reasons:

[1] This is a solution to a problem that may never occur (at least in our lifetimes). While nothing lasts forever, my guess is that github is gonna be around for quite some time.

[2] Should this problem occur, there will be lots of work to do, and I think the most trivial part of that will be correcting the URLs. All references to them would have to be re-evaluated anyway. I see no need to make any initial pass at them now.

[3] If there is any value in fixing the URLs, I am not convinced that using #Defines and #Includes is the way to go and that it works well in the Thor environment. I am not saying that it's wrong, just that I would want to evaluate alternatives to make sure that it's the best approach. (I for one am not a fan of #Defines/#Includes and have concerns about how they would work in the Thor environment, most of which is in open PRGs, not APPs.)

[4] Should there come a time to implement this (and I see no compelling reason to do so), I certainly would not want to make a mass change to such a large number of Thor files. The time to adequately test all of this would be staggering and the risk of breaking Thor in the meantime would be considerable.

lscheffler commented 9 months ago

It would have been possible to read the linked CONTRIBUTING, or simple look up the test provided. Or simple run a search tool like Code References over Thor to see where URL's are used at all. The strings "/Thor" and "/master" or "http" / "https" might do.

Nobody says blindly merge.

Pro tip: installed files\thor\tools\procs\thor_proc_check_for_updates.prg oops. A DEFINE for an URL ... Seems not only to be me to have this idea. Probably one might ask the person doing the change changed URL from VFPXRepository to GitHub about the fun doing this over and over - it even looks like this is a good search string to identify the places pointing to Thor or ThorRepository repo.