#28086 closed enhancement (fixed)

MR27: Remove use of installed_packages for threejs

Reported by: galois Owned by:
Priority: major Milestone: sage-8.9
Component: graphics Keywords: threejs
Cc: saraedum, paulmasson, fbissey, arojas, gh-timokau Merged in:
Authors: Isuru Fernando Reviewers: Paul Masson
Report Upstream: N/A Work issues:
Branch: c643b25 (Commits) Commit: c643b25b49791d9467be734d5689b313ddf347b6
Dependencies: #28007 Stopgaps:

Change History (15)

comment:1 Changed 15 months ago by isuruf

  • Cc saraedum paulmasson added
  • Component changed from PLEASE CHANGE to graphics
  • Description modified (diff)
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 15 months ago by galois

  • Commit changed from 97fe1751147853313c3d010bd4346104882c4c64 to 1b17e523b3f132856411bbeb1771757d69497f56

New commits added to merge request. I updated the commit SHA-1. New commits:

1b17e52add missing import os

comment:3 Changed 15 months ago by galois

  • Commit changed from 1b17e523b3f132856411bbeb1771757d69497f56 to ad4e8a90cd9a44b334a78c9d1d1d67f76484229c

New commits added to merge request. I updated the commit SHA-1. New commits:

ad4e8a9Add r to version

comment:4 Changed 15 months ago by paulmasson

  • Cc fbissey arojas added

While this will certainly work, what about a global variable THREEJS_VER evaluated just once?

comment:5 Changed 15 months ago by fbissey

I am rather indifferent on how this is implemented. But even if create a variable THREEJS_VER in env.py it will be evaluated each time you call? Or are you thinking of hard-coding the version? I am not a big fan of hardcoding myself, is it really that detrimental?

comment:6 Changed 15 months ago by paulmasson

Was thinking env.py only ran once, but if it runs ever time it is called then there will be no difference.

comment:7 Changed 15 months ago by paulmasson

  • Dependencies set to #28007
  • Keywords threejs added
  • Reviewers set to Paul Masson
  • Status changed from needs_review to positive_review

Works for me

comment:8 Changed 15 months ago by chapoton

  • Status changed from positive_review to needs_work

Patchbot reports several doctest failures, guys..

And patchbot plugins are not green either.

comment:9 Changed 15 months ago by fbissey

That's because the dependency at #28007 is merged and closed but not in any release or beta yet. I am surprised it merges on the patchbots.

comment:10 Changed 15 months ago by gh-timokau

  • Cc gh-timokau added

comment:11 Changed 15 months ago by chapoton

ok then, but still the plugins "pyflakes" and "pycodestyle" are not green.

comment:12 Changed 15 months ago by galois

  • Commit changed from ad4e8a90cd9a44b334a78c9d1d1d67f76484229c to c643b25b49791d9467be734d5689b313ddf347b6

New commits added to merge request. I updated the commit SHA-1. New commits:

c643b25Fix formatting error

comment:13 Changed 15 months ago by isuruf

  • Status changed from needs_work to needs_review

pyflakes error is for a line I didn't change. I've fixed pycodestyle error.

comment:14 Changed 15 months ago by paulmasson

  • Status changed from needs_review to positive_review

Any good reason not to set this back to positive review?

comment:15 Changed 15 months ago by vbraun

  • Branch changed from u/galois/mrs/27/threejs to c643b25b49791d9467be734d5689b313ddf347b6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.