Opened 4 years ago

Last modified 17 months ago

#21309 closed enhancement

Package thebe.js — at Version 20

Reported by: tmonteil Owned by:
Priority: major Milestone: sage-7.5
Component: packages: standard Keywords: sd75, sdl
Cc: fcayre, vdelecroix, nthiery, slelievre Merged in:
Authors: Thierry Monteil Reviewers: Nicolas M. Thiéry
Report Upstream: N/A Work issues:
Branch: u/tmonteil/package_thebe_js (Commits) Commit: 520cdd0796488eebc64c3796e7030504ab941661
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

With the first implementation of #20690, thebe.js was entangled within Sage source code. Since upstream is well defined, the aim of this ticket is to package it the usual way. #20690 is now based on it.

The zipball can be found at https://github.com/oreillymedia/thebe/archive/9624e0a07a00026103dce1a3e32bbfbf90a6d0f9.zip and should be renamed thebe-9624e0a0.zip (upstream does not propose explicit releases).

It can also temporarly be downloaded at https://lipn.univ-paris13.fr/~monteil/hebergement/tmp/thebe-9624e0a0.zip

Change History (20)

comment:1 Changed 4 years ago by tmonteil

  • Branch set to u/tmonteil/package_thebe_js

comment:2 Changed 4 years ago by tmonteil

  • Cc fcayre vdelecroix nthiery slelievre added
  • Commit set to 7d4fe864b00482ed97a0fc1f98a8150e5c83fc84
  • Status changed from new to needs_review

New commits:

7d4fe86#21309 : package thebejs.

comment:3 Changed 4 years ago by tmonteil

  • Keywords sd75 added

comment:4 Changed 4 years ago by nthiery

  • Branch changed from u/tmonteil/package_thebe_js to u/nthiery/package_thebe_js

comment:5 Changed 4 years ago by git

  • Commit changed from 7d4fe864b00482ed97a0fc1f98a8150e5c83fc84 to e1b914a7ec935bd6fadb8eb1cd8f6ced77b26bd5

Branch pushed to git repo; I updated commit sha1. New commits:

e1b914a21309: made the instructions to fetch the latest thebe version into an executable shell command

comment:6 Changed 4 years ago by nthiery

  • Description modified (diff)
  • Reviewers set to Nicolas M. Thiéry
  • Status changed from needs_review to positive_review

We discussed this face to face with Thierry, and I made minor improvements to the text in the SPKG that he double checked over my shoulder. The files look good. I tested it, and it works.

Positive review. Thanks Thierry!

comment:7 Changed 4 years ago by fbissey

I personally would have preferred a spkg-src that pulls a given git commit. Downloading master.zip is quite likely not a repeatable operation (as in "it doesn't necessarily give you the same file every time").

comment:8 Changed 4 years ago by tmonteil

Would downloading thebe-9624e0a07a00026103dce1a3e32bbfbf90a6d0f9.zip instead of master.zip be satisfying ?

comment:9 Changed 4 years ago by fbissey

It would probably work :)

comment:10 Changed 4 years ago by tmonteil

  • Status changed from positive_review to needs_work

comment:11 Changed 4 years ago by tmonteil

  • Branch changed from u/nthiery/package_thebe_js to u/tmonteil/package_thebe_js

comment:12 Changed 4 years ago by tmonteil

  • Commit changed from e1b914a7ec935bd6fadb8eb1cd8f6ced77b26bd5 to 8af8f5e03450b475650b7177f243eb49e46b2808
  • Description modified (diff)
  • Status changed from needs_work to needs_review

OK i have done that. The hash of the zipball changed, but according to dirdiff the content is the same.


New commits:

8af8f5e#21309 : fetch the archive named by the commit of the master branch.

comment:13 Changed 4 years ago by vbraun

  • Milestone changed from sage-7.4 to sage-7.5

comment:14 Changed 4 years ago by tmonteil

Is it satisfactory like this ?

comment:15 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

You should have the git commit in the version number. It should be possible to exactly reconstruct the tarball. The date on which it was packaged is not sufficient for that.

When you do this, also adjust the instructions in SPKG.txt (and perhaps replace #20690 by the actual Trac URL).

comment:16 follow-up: Changed 4 years ago by jdemeyer

Also: upstream calls itself thebe, why rename to thebejs?

comment:17 Changed 4 years ago by nthiery

Hi Thierry! Just to avoid duplicated work: have you started implementing Jeroen's comments? otherwise I can have a go tomorrow morning with him.

comment:18 Changed 4 years ago by git

  • Commit changed from 8af8f5e03450b475650b7177f243eb49e46b2808 to 520cdd0796488eebc64c3796e7030504ab941661

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

520cdd0#21309 : thebejs -> thebe ; use hash in pkg name ; link to trac URL.

comment:19 in reply to: ↑ 16 Changed 4 years ago by tmonteil

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Replying to jdemeyer:

You should have the git commit in the version number. It should be possible to exactly reconstruct the tarball. The date on which it was packaged is not sufficient for that.

Done.

When you do this, also adjust the instructions in SPKG.txt (and perhaps replace #20690 by the actual Trac URL).

Done.

Replying to jdemeyer:

Also: upstream calls itself thebe, why rename to thebejs?

Most javascript libs have dual names, for example node vs nodejs or d3 vs d3js. Since we used the *js name for in other packages, i took that one, but indeed, in the case of thebe there is no *js version. So i renamed the package thebeas suggested.

comment:20 Changed 4 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.