Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MERGE AFTER 28/7] Remove curve library. #1683

Merged
merged 11 commits into from
Jul 30, 2021
Merged

[MERGE AFTER 28/7] Remove curve library. #1683

merged 11 commits into from
Jul 30, 2021

Conversation

yongxiangng
Copy link
Contributor

Description

Remove curve related code.

  • Remove file public/externalLibs/graphics/webGLcurve.js (curve renders & code related to generating curve)
  • Remove file public/externalLibs/graphics/webGLhi_graph_ce.js (curve transformations + code from repeat module)
  • Remove relevant code from src/commons/application/types/ExternalTypes.ts (curve library & functions exposed in curve library, Intentionally left CURVES = 'CURVES', intact (line 14) to prevent errors in other files)
  • Remove curve related code from public/externalLibs/graphics/webGLgraphics.js
  • cadet-frontend/public/externalLibs/index.js no longer loads webGlhi_graph_ce and webGlcurve when loading all libs

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

Any existing code which tries to load curve library will fail because it does not exist anymore. I have left the ExternalLibraryName.CURVES as it is to prevent any errors in those files affected (e.g. AssessmentMocks.ts, WorkspaceSaga.ts)

Rune library should however work as before. (https://source-academy.github.io/source/RUNES/)
i.e. these functions should work

    anaglyph
    beside
    beside_frac
    black
    blank
    blue
    brown
    circle
    color
    corner
    flip_horiz
    flip_vert
    green
    heart
    hollusion
    indigo
    make_cross
    nova
    orange
    overlay
    overlay_frac
    pentagram
    pink
    purple
    quarter_turn_left
    quarter_turn_right
    random_color
    rcross
    red
    repeat_pattern
    ribbon
    rotate
    sail
    scale
    scale_independent
    show
    square
    stack
    stack_frac
    stackn
    translate
    turn_upside_down
    white
    yellow

These are the runes I used for testing. Other libraries are able to load, including all libraries, but I did not extensively test other libraries other than runes as they are unlikely to be affected.

anaglyph(heart);
show(beside(heart, heart));
show(beside_frac(0.2,heart, heart));
show(black(red(heart)));
show(blank);
show(blue(heart));
show(brown(heart));
show(red(circle));
show(corner);
show(flip_horiz(corner));
show(flip_vert(corner));
show(green(heart));
hollusion(heart);
hollusion(indigo(heart));
hollusion(indigo(make_cross(heart)));
show(nova);
show(orange(nova));
show(overlay(corner, nova));
show(overlay_frac(0.3, corner, nova));
show(red(pentagram));
show(pink(pentagram));
show(purple(pentagram));
show(quarter_turn_left(purple(heart)));
show(quarter_turn_right(purple(heart)));
show(quarter_turn_right(random_color(heart)));
show(rcross);
show(red(rcross));
show(repeat_pattern(2, x => beside(x, circle), heart));
show(ribbon);
show(rotate(math_PI/3 ,heart));
show(sail);
show(scale(0.25, sail));
show(scale_independent(0.25, 1, sail));
show(square);
show(stack(heart, red(square)));
show(stackn(5, heart));
show(translate(0.2,0.3,heart));
show(turn_upside_down(heart));
show(white(heart));
show(yellow(heart));

Documentation has been ported to modules repository together with the relevant code.

Checklist

  • I have tested this code
  • I have updated the documentation

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 3, 2021

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0ee4a08
Status: ✅  Deploy successful!
Preview URL: https://91ab70a0.cadet-frontend.pages.dev

View logs

@coveralls
Copy link

coveralls commented Apr 3, 2021

Pull Request Test Coverage Report for Build 859223778

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.006%) to 29.693%

Totals Coverage Status
Change from base Build 847480317: -0.006%
Covered Lines: 2707
Relevant Lines: 8394

💛 - Coveralls

@martin-henz martin-henz marked this pull request as draft April 5, 2021 05:33
@martin-henz
Copy link
Member

converting to Draft because we need to adapt the teaching material. When the missions are converted to the new curves module, we can merge this PR.

@martin-henz
Copy link
Member

This PR should not be merged earlier than 28/7 (the date of the final assessment of the SICP summer course)

@martin-henz martin-henz changed the title Remove curve library. [MERGE AFTER 28/7] Remove curve library. Jun 29, 2021
@coveralls
Copy link

coveralls commented Jun 29, 2021

Pull Request Test Coverage Report for Build 982289385

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.005%) to 30.026%

Totals Coverage Status
Change from base Build 977492937: -0.005%
Covered Lines: 3198
Relevant Lines: 9805

💛 - Coveralls

@martin-henz martin-henz marked this pull request as ready for review July 30, 2021 04:11
@martin-henz martin-henz merged commit 7228de9 into master Jul 30, 2021
@jiayushe jiayushe deleted the sa2122-graphics branch September 7, 2021 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants