This repository was archived by the owner on Nov 2, 2023. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 214
Audit validator list, compare w/test suite #154
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gives me slight pause. The implementation language is CoffeeScript, but the platform/environment is JavaScript (see also .NET, implementation language in all cases is C#).
My main concern would be discoverability: it could be used by people looking for a JavaScript solution. The same would apply for TypeScript (should it ever arise).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamvoss Those are good points, but this PR does not make the problem worse, so I'd rather address them separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless you are saying that .NET is wrong and should be changed, this PR introduces the problem (in this section of the software page anyway).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, I was flipping .NET and C#'s roles in my head. Probably because I never think about them at all.
I would expect to find CoffeeScript as its own thing. It's hardly the first language to "compile" to another language (I am old enough to have compiled C++ with CFRONT, albeit only for compatibility tests).
Explaining to JavaScript programmers that they could use CoffeeScript (or.. ick.. TypeScript) is not our responsibility. It's more important that CoffeeScript/TypeScript users can find their own implementations.
I suppose this conflicts with how .NET is done, technically, but I assume it's done that way because the .NET/C# people thought that was better. I'm not really bothered by the inconsistency.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .NET libraries in question can be consumed by any .NET-platform language, the fact they are implemented in C# is irrelevant from the standpoint of the consumer. On the other hand, if there were a F# (which is a .NET language) implementation that did not put in additional effort of be compatible it would best be listed as F# because it could not be consumed by other .NET languages in a sensible way (this is similar to how the JVM-based languages are correctly listed separately here)-.
In the JavaScript/TypeScript/CoffeeScript world, the same compatibility exists. If I am writing in JavaScript/TypeScript/CoffeeScript, I am still going to go to NPM to download my dependencies where I could download jsck. It will work just fine and I might not even learn that it was written in CoffeeScript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just not a central enough concern to the JSON Schema project for me to want to hold up a PR and debate it. If you are set on blocking this, that's fine and I'll drop it, but I'm not reworking it or debating appropriate language classifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamvoss the .NET people added their stuff under .NET, and the first CoffeeScript person added theirs under CoffeeScript (in the test suite repo, which motivated this change). I'm not inclined to contradict either of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced this is an issue, and suggest if you feel stongly about it, open a new issue.