Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Point at toml-test #11

Merged
merged 1 commit into from
Oct 3, 2022
Merged

Point at toml-test #11

merged 1 commit into from
Oct 3, 2022

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Jul 31, 2022

We could push toml-test over this (maybe even force-push so history is
preserved), but then we'd have two repos and that's confusing.

The best case would be to move/rename BurntSushi/toml-test to here; I
think GitHub will then redirect BurntSushi/toml-test, but not 100%
sure about that, and if it will remain working for "deep links" to files
or issue comments etc.

Also, there's the issue with repo/organisation permissions.

Anyway, to avoid confusion and double work (like #8) just point to the
toml-test repo in the README. I think that's probably also the best
long-term solution; that repo has been around for a decade, and renaming
things tends to cause more problems than it solves.

Closes #8

We could push toml-test over this (maybe even force-push so history is
preserved), but then we'd have two repos and that's confusing.

The best case would be to move/rename BurntSushi/toml-test to here; I
*think* GitHub will then redirect BurntSushi/toml-test, but not 100%
sure about that, and if it will remain working for "deep links" to files
or issue comments etc.

Also, there's the issue with repo/organisation permissions.

Anyway, to avoid confusion and double work (like #8) just point to the
toml-test repo in the README. I think that's probably also the best
long-term solution; that repo has been around for a decade, and renaming
things tends to cause more problems than it solves.

Closes #8
@arp242
Copy link
Contributor Author

arp242 commented Jul 31, 2022

@pradyunsg

@eksortso
Copy link

@arp242 I personally find it vicious to suggest that all the work already done in this repository ought to be erased. This project exists for reasons unrelated to the existence of toml-test, and you address none of that history. I wouldn't be surprised to find this PR summarily closed.

@arp242
Copy link
Contributor Author

arp242 commented Jul 31, 2022

I personally find it vicious to suggest that all the work already done in this repository ought to be erased.

But right now it's little more than an empty placeholder? The only thing is a PR that's been open for a year and a half, which basically just copies a bunch of (already existing) test files from iana's (seemingly unmaintained) "upstream" repo. All of that actually needs quite a lot of work to be anywhere near as comprehensive as what toml-test is today.

I find "vicious" to be a rather curious choice of words, but okay... This isn't randomly out of the blue, I discussed it a year ago and it was generally received positively, with no one objecting.

At any rate, what would your suggestion be? Maintain two test solutions which are largely overlapping? And who is going to maintain this one?

AFAIK the only reason this was created is because BurntSushi stopped maintaining toml-test and it's outdated, but this situation has been resolved for over a year now.

@arp242
Copy link
Contributor Author

arp242 commented Jul 31, 2022

The goal I'm trying to achieve here is that folks implementing (new) TOML libraries can easily and quickly find a good TOML test suite. I don't care which way we go about that, but I do know is that now people will end up here and won't find anything, and that this has been the situation for years. This is far from ideal and it seems to me that this PR achieve the goal reasonably well, also keeping in mind the many implementations that already use BurntSushi/toml-test and to not break any of that (or introduce "churn" by having them switch to something else).

I'm a bit taken aback by your response to be honest; obviously I have no interest in erasing anyone's work, but I also have no interest in spending many more days getting this repo up-to-spec, especially considering I (and others: moorereason, sgarciac, and more) already spent a lot of time doing exactly the same work over last year and a half, which – by the way – involved going through all of iana's tests that #8 adds here one-by-one to ensure we didn't end up with duplicates and that they were correct.

@eksortso
Copy link

Point taken. In my defense, you could have linked back to that conversation when you opened the PR. Without the context, it seemed like your actions were sweeping and unilateral, not the sort of "vicious" thing that's kosher in open-source projects. I'm sorry that I jumped at you. I'll stand down and let the project leads have their say.

@arp242
Copy link
Contributor Author

arp242 commented Aug 1, 2022

Yeah, I definitely could have included more context in the original PR description; I guess I just kinda assumed people are already familiar with it 😅

@CAM-Gerlach
Copy link

The best case would be to move/rename BurntSushi/toml-test to here; I
think GitHub will then redirect BurntSushi/toml-test, but not 100%
sure about that, and if it will remain working for "deep links" to files
or issue comments etc.

That will work; I've moved a number of repos between orgs in the past with no problems at all, and everything existing (from Git config to all types of links) working seamlessly. I've tested it right now on a variety of deep links (commits, PRs, comments, files, raw files, etc) on those repos but using the old URL, and they all worked perfectly; it just rewrites the org part of the link.

It would just need to be moved to a different name, or this repo moved first (and then archived, as you can also do after this is merged) to avoid loosing everything here and avoid breaking existing inbound links to this repo.

You could add the current maintainer(s) of the BurntShushi tests as repo-level Collaborators with Maintain or Admin permissions, whatever they currently have now.

This avoids any future issues with the tests over there on some person's personal account going unmaintained or even vanishing completely due to the owner becoming inactive, as well as making them an official part of the TOML project and directing additional user and contributor attention to them.

@arp242
Copy link
Contributor Author

arp242 commented Aug 2, 2022

This avoids any future issues with the tests over there on some person's personal account going unmaintained or even vanishing completely due to the owner becoming inactive, as well as making them an official part of the TOML project and directing additional user and contributor attention to them.

Yes, the current situation is a bit odd: BurntSushi is the original creator, but he hasn't been involved in quite a few years, and I've since taken over. So it's dependent on two people: me to maintain it, BurntSushi not to delete his account or whatnot (or to add new people, if need be), etc. The reason people started working on this and other solutions is because it was unmaintained for a few years before I took over.

But the situation on the toml-lang repo isn't all that much better 😅 See toml-lang/toml#895 and toml-lang/toml#585 (comment)

Actually, come to think of it, moving BurntSushi/toml-test to toml-lang/toml-test and archiving this is probably the best, because we'd have to retain the toml-test command name anyway since renaming that would be needless breakage IMO. We can then just update the README to point at that here (so this PR still stays relevant).

@CAM-Gerlach
Copy link

Yes, the current situation is a bit odd

Thanks for the background!

But the situation on the toml-lang repo isn't all that much better sweat_smile See toml-lang/toml#895 and toml-lang/toml#585 (comment)

Yeah, though seems like there's at least the potential for that to get better soon, based on the discussion on the issue so far, whereas with a user account repo you're stuck with a bus factor of 1 forever unless and until you move it to an org account.

Actually, come to think of it, moving BurntSushi/toml-test to toml-lang/toml-test and archiving this is probably the best, because we'd have to retain the toml-test command name anyway since renaming that would be needless breakage IMO. We can then just update the README to point at that here (so this PR still stays relevant).

Yeah, something like that was basically my (non-expert) recommendation.

@pradyunsg
Copy link
Member

Let's do this -- I'd still prefer to move toml-test over, but that's a logistical problem for a future us.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants