DefinitelyTyped / NugetAutomation

Automatically generate nuget packages for the DefinetlyTyped TypeScript definitions.
Other
26 stars 10 forks source link

Create different package/assembly for semvers (& legacy) #12

Open Bartvds opened 10 years ago

Bartvds commented 10 years ago

As per https://github.com/borisyankov/DefinitelyTyped/pull/2423 (see also https://github.com/borisyankov/DefinitelyTyped/issues/2207 and https://github.com/borisyankov/DefinitelyTyped/issues/2288)

It'd solve some user problems if a separate package was created for semver postfixed files (and their base).

Main case is node.d.ts & node-0.8.8.d.ts from https://github.com/borisyankov/DefinitelyTyped/tree/master/node

So basically it matched the base and semver version and sees the different versions of same thing.

More info http://semver.org/

Sometimes there are only 2 digits (foo-0.9) so some extra logic can be needed to try and patch-up the semver.

For TSD I use this chunky logic: https://github.com/DefinitelyTyped/tsd/blob/dev/next/src/tsd/data/Def.ts#L60-L98 (this uses semver npm module but it could be a lot simpler I guess).

Note it should probably also covers the magic legacy folder, like for https://github.com/borisyankov/DefinitelyTyped/tree/master/angularjs and https://github.com/borisyankov/DefinitelyTyped/tree/master/jasmine

Bartvds commented 10 years ago

ping @staxmanade :wink:

staxmanade commented 10 years ago

...response timed out...

vvakame commented 10 years ago

lol

basarat commented 10 years ago

:+1: participating

staxmanade commented 10 years ago

I have a partial solution - just exclude all sem-ver'd files for now. Let ppl manually grab older versions. Any objections to this (it at least un-blocks) people from using NuGet to pull in the latest.

Bartvds commented 10 years ago

I guess it'll do. If it is a big problem we'll get notified in the Issues.

You can also exclude legacy & releases subfolders (like for angularjs). But include the other subdirectories, there are a few defs that are use them (like d3 has a plugins subfolder).

johnnyreilly commented 10 years ago

Works for me :+1:

staxmanade commented 10 years ago

We've handled the legacy folder for a while - https://github.com/DefinitelyTyped/NugetAutomation/blob/master/CreatePackages.ps1#L137

I don't see anything with a releases folder. Can you point me to an example so I can test it out before I merge this in and re-build all the packages at once?

Bartvds commented 10 years ago

Nice. There were defs in https://github.com/borisyankov/DefinitelyTyped/tree/master/winjs with a releases but they got moved so maybe it's not yet necessary.

But I now realise maybe the semver files do need a package (instead of ignoring): I just recall the winjs ones are supposedly all active, as 1.x and 2.x are both still under development. See https://github.com/borisyankov/DefinitelyTyped/pull/2423

staxmanade commented 10 years ago

@Bartvds my initial concern on supporting the semver was the complexity that could crop up - I don't know all of the possible options, but could we go as simple as "if it's a semver'd file - package that up all by itself?"

What about just moving the semver'd files out into their own folders? So instead of

winjs
    winjs-1.0.d.ts
    winjs-2.0.d.ts

we could have

winjs-1.0
    winjs-1.0.d.ts
winjs-2.0
    winjs-2.0.d.ts

I know it could blow up the root of the repo... (but could be easier to manage?)

staxmanade commented 10 years ago

The more I think about it - the more this pattern make more sense across the board...

winjs-1.0
    winjs-1.0.d.ts
winjs-2.0
    winjs-2.0.d.ts
Bartvds commented 10 years ago

Does NuGet have a facility to specify different views of same package? Like versions or assemblies or something like that?

basarat commented 10 years ago

Does NuGet have a facility to specify different views of same package?

Yes. But its like npm. Continuous incrementing numbers. We can't really use them to version packages to match lib versions.

the more this pattern make more sense across the board.

:+1:

johnnyreilly commented 10 years ago

@staxmanade I think that is a great idea.

If I put my users hat on for a minute I know that when Angular 1.3 (and for that matter 2.0) ship I'll be using those in some projects whilst I'll be using 1.2 in projects that require backwards compatibility. For that reason it'll be good to have separate semver'd versions which can be maintained going forwards.

Switching to that mechanism would also be a nice and clear "break point" with the current mechanism as well. Pre-semver'd packages look like this in Nuget:

- angularjs.TypeScript.DefinitelyTyped

Post-semver'd packages look like this:

- angularjs-1.2.TypeScript.DefinitelyTyped
- angularjs-1.3.TypeScript.DefinitelyTyped
- angularjs-2.0.TypeScript.DefinitelyTyped

This sounds like a good solution to me.

basarat commented 10 years ago

angularjs.TypeScript.DefinitelyTyped will continue to exist. It will be the HEAD version.

johnnyreilly commented 10 years ago

Hi @Basarat,

So let's say Angular 2.0 had shipped - we'd have:

- angularjs.TypeScript.DefinitelyTyped
- angularjs-1.2.TypeScript.DefinitelyTyped
- angularjs-1.3.TypeScript.DefinitelyTyped

From a folder structure like this:

angular
    angular.d.ts
    angular-1.2 // folder
        angular-1.2.d.ts
    angular-1.3 // folder
        angular-1.3.d.ts

With typings sitting in the root and also in the subfolders. The root would be the Angular 2.0 typings (until say Angular 2.1 shipped and a new angular-2.0 folder is created)

Sounds fair. Though if we have folder names indicating semvers do we also need them in filenames too? I'd prefer this:

angular
    angular.d.ts
    angular-1.2 // folder
        angular.d.ts
    angular-1.3 // folder
        angular.d.ts

Any reason to favour semver in filename as well as folder name?

basarat commented 10 years ago

Any reason to favour semver in filename as well as folder name?

Actually I was thinking semver in filename only. That's just because that's how tsd works (and I like it).

angular  // folder
    angular.d.ts
    legacy // folder
        angular-1.2.d.ts
        angular-1.3.d.ts

legacy because angular is split in multiple files. For node (https://github.com/borisyankov/DefinitelyTyped/tree/master/node) its just:

node  // folder 
   node.d.ts
   node-0.8.8.d.ts
Bartvds commented 10 years ago

Agree with @basarat, mostly because that is what we use already.

johnnyreilly commented 10 years ago

It occurs to me that I've been assuming that in this brave new world we'd also run our existing tests on the legacy typings as well. That's kind of why I favoured no filename change as we wouldn't have to go through and change the references in the typings. (Not that it's a massive hardship)

Are other people thinking the same re: running tests on legacy (but still maintained) typings? I think there's value in it if we're going to publish NuGet packages off the back of it.

Also, on the single folder thing, don't we have a bit of logical grouping problem? To extend our example a little further:

angular  // folder
    angular.d.ts
    angular-animate.d.ts
    angular-cookies.d.ts
    angular-mocks.d.ts
    angular-resource.d.ts
    angular-route.d.ts
    angular-sanitize.d.ts
   legacy // folder
        angular-animate-1.2.d.ts
        angular-animate-1.3.d.ts
        angular-cookies-1.2.d.ts
        angular-cookies-1.3.d.ts
        angular-mocks-1.2.d.ts
        angular-mocks-1.3.d.ts
        angular-resource-1.2.d.ts
        angular-resource-1.3.d.ts
        angular-route-1.2.d.ts
        angular-route-1.3.d.ts
        angular-sanitize-1.2.d.ts
        angular-sanitize-1.3.d.ts
        angular-1.2.d.ts
        angular-1.3.d.ts

The different versions all get mingled together where a typing is spread over several files. Whilst it's copable with I'd tend towards something that grouped each version together (fall into the "pit of success" and all that jazz).

Bartvds commented 10 years ago

The tester supposedly runs on the legacy stuff, same as the main defs. But the tests refer to the definitions they are testing, so there is no magic "run our existing tests on the legacy typings as well".

AngularJS is a bit of an oddball in that it has many sub-projects in one folder. On TSD you got to install them separately (unless you use a pattern).

If it has to be changed simplify some more, and every /^\d+\.\d+/ will be a legacy folder.

angular  // folder
    angular.d.ts
    1.2  // folder
        angular.d.ts
    1.3  // folder
        angular.d.ts         

I'm not sure if this is a real problem though.

johnnyreilly commented 10 years ago

I see what you mean @Bartvds. Though (unlike Node I guess - but haven't checked) each typing ideally consists of at least 2 files:

iAmATyping.d.ts
iAmATyping-tests.ts

Let's pretend Angular was just 1 file. We'd still have:

angular  // folder
   angular.d.ts
   angular-tests.ts
   legacy // folder
        angular-tests-1.2.ts
        angular-tests-1.3.ts
        angular-1.2.d.ts
        angular-1.3.d.ts

The grouping problem remains (though this is not a massive issue I agree :smile:)

To cater for the edge cases where you have examples like Angular where it is multiple files then folders is still what I'd prefer. I think in the long run it's a choice that might be better from a code organisation point of view. Just my 10 cents!

Bartvds commented 10 years ago

Yea but with -tests after the name, so we can (crudely) match tests to defs and report them in the tester.

angular  // folder
   angular.d.ts
   angular-tests.ts
   legacy // folder
        angular-1.2-tests.ts
        angular-1.3-tests.ts
        angular-1.2.d.ts
        angular-1.3.d.ts

I'm not impartial to using semver folders. The TSD update that has the clever /legacy support is still sitting locally here so that is not blocking it.

But if we go for this then we'd move all existing stuff too and maybe even enforce it (eg: deny legacy/releases folder), so we'd better make sure we want it.

johnnyreilly commented 10 years ago

For my part folders is a preference - not a must have.

What do other people think? So far I count @staxmanade and I favouring folders per version and @basarat and @Bartvds favouring a single legacy folder approach.

Bartvds commented 10 years ago

Also ping @borisyankov as he is not listed as repo watcher.

borisyankov commented 10 years ago

I'm more for a single folder too.

staxmanade commented 10 years ago

I'm not happy with the complexity that having multiple versions within the same folder brings up. If we could guarantee a single file per project then we could make it work, but I don't think that's a good assumption to work off of.

It would make it simpler to maintain and easier to see the logical grouping via folders. I would really like to push to use one of the following structures.

A:

rootOfDT/
    projectA
          projectA.d.ts
    projectA-1.2
          projectA-1.2.d.ts

B:

rootOfDT/
    projectA
          projectA.d.ts
          legacy
              projectA-1.2
                  projectA-1.2.d.ts

I've not been a fan of the legacy folder for a couple reasons:

I'd prefer A: because that would not require any modifications to the long-running nuget automation that is already in place.

In the end what I want to see is

johnnyreilly commented 10 years ago

I have to say that I'm totally sold on the points made by @staxmanade. They make sense to me and I'd go with A or B quite happily.

Maybe one of @basarat / @Bartvds / @borisyankov could express the benefits of the single folder approach instead of @staxmanade suggestions. I think single folder was favoured because this approach is like tsd. Are there other reasons to favour a single folder approach as well?

borisyankov commented 10 years ago

My preference for the root = current, legacy = everything else, comes only from the view point of the user. When they go to the folder, it is obvious what they need, and if they need the latest, they don't need anything in legacy.

The projectA vs projectA-1.2 would be confusing, as I am sure many would not know why projectA does not have a version number. Is it the latest? Is it something more general? Is it the first?

Including version numbers in all names, like projectA-1.3 and projectA-1.2 would make this confusion go away, but now we will need to have versions everywhere.

We certainly need to make it both unambiguous for automation, and obvious for the user.

Bartvds commented 10 years ago

The legacy stuff and semver postfxed files are already being used for a long while (at least since node 0.10 came out), it's just that the NuGet exporter ignored it.

For TSD I have this logic working; it is just some RegExp and loops (maybe easier in JS then PowerShell?). Also note TSD doesn't 'package' per folder but per file-name and then solves <references>.

For @staxmanade 's options:

Definitely not A, it will make a huge mess of the repo by creating even more folders in the root. It is bad enough as it is.

I don't like B either as it creates too much nested folders, and the semver postfix is repeated in both the folder and the filename.

If we really must split per version then I like this better, as it is shallow, simple and less repetitive:

rootOfDT/
    projectA
          projectA.d.ts
          1.2
              projectA.d.ts
          2.3
              projectA.d.ts
johnnyreilly commented 10 years ago

I see what @Bartvds means about option A - that's a fair point about clutter in the root. I agree.

I still like option B but I agree about it being unnecessary to have the semver in both file and folder. (And maybe it is too deep a folder structure)

I like @Bartvds suggestion for the following reasons:

  1. It's quite simple
  2. It's pretty clear from a user perspective (which @borisyankov rightly points out is a concern)

@staxmanade what do you think?

staxmanade commented 10 years ago

I like where @Bartvds proposal is going. Would like to tweak it just a little. Could we push the version folders down inside the legacy folder?

Something like this would be easier to disambiguate as we can easily ignore the legacy folder when packaging up the project - and then iterating the legacy/* folders for the semver'd packages...

rootOfDT/
    projectA/
          projectA.d.ts
          legacy/
              1.2
                  projectA.d.ts
              2.3
                  projectA.d.ts 
Bartvds commented 10 years ago

Mjah, is that really necessary? A simple regexp like /^\d+\.\d+/ can determine if it is a semver-ish folder.

staxmanade commented 10 years ago

I guess my assumption of the original structure of the repo (may be out of date) but was that all files in a folder belong to the 'package'. So in the case of angularjs all of the *.d.ts files come down in one package. Similarly having the 1.x version folders in there muddy up the logic when determining what sub-folders to include/exclude of a package. In cordova for example, there is a plugins folder. Given my approach above, and grouping by folders, makes the management of these structures much simpler.

Bartvds commented 10 years ago

I'm not aware of formal defined meanings in any of this, so I made TSD support both per-file as per-folder and optional reference solving :smile:

For NuGet just loop the folder names inside the project's folder, if it matches the regexp it is a legacy version of package, otherwise it is part of the content. Can't be that complex right?

It is only a few lines of logic, and it would simplify the repo if it doesn't have glue folders.

basarat commented 10 years ago

I like the folder semver naming since it would make sending a PR that changes something drastically (due to next version) move the old stuff to folder x.x and not care about renaming files (which is a bit of a pain as you need to fix up the reference tags as well). I guess we are all decided on folder for semver so that's finalized (unless someone disagrees).


The only question is where do we put these semver'ed folders. My vote legacy.

ReasonA: consider package foo (with massive submodules bar and baz) :

rootOfDT/
    foo/
          foo.d.ts
          bar/
               bar1.d.ts
               bar2.d.ts
          bas/
               bas1.d.ts
               bas2.d.ts
          legacy/
              1.2
                  foo.d.ts
                      bar/
                           bar1.d.ts
                           bar2.d.ts
                      bas/
                           bas1.d.ts
                           bas2.d.ts
              2.3
                  foo.d.ts
                      bar/
                           bar1.d.ts
                           bar2.d.ts
                      bas/
                           bas1.d.ts
                           bas2.d.ts

Reason B: It also goes with what @borisyankov likes:

if they need the latest, they don't need anything in legacy.

Reason C: Also helps with preventing people trying to send a PR to to a legacy definition e.g. angular 1.2, if they find a bug in the definition, just because that the version of angular they are using and think that's the definition version where the fix belongs (after all, if they fetched latest all they got was angular.d.ts from nuget / tsd with no indication of its location on DT)

johnnyreilly commented 10 years ago

I've happy with these proposals. @basarat - Reason C makes a lot of sense.

Bartvds commented 10 years ago

Does all this also work for definitions that have multiple current versions?

For example a case in https://github.com/borisyankov/DefinitelyTyped/issues/2293 where the described project has still active development on both 1.x and 2.x.

The nice thing of having all of them in one folder is how they are all visible.

One other options is to move everything into versioned folders, so instead of having one main version and a bunch of legacy sub folders move stuff like:

/1.0
/2.0

There could also be a 'latest' folder, like

/1.0
/2.0
/latest

But that'd be confusing, maybe not do that.

Instead just move everything in a numbered version folder (including the 'latest' project/module.d.ts that is now not numbered).

Again, I kinda like the way we have it know, it is simple for users and has the options (but not requirement) of putting stuff out of sight in /legacy when it is out of date.(maybe rename to /versions instead (or /releases).