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

Fixed the step options being set in an infinite loop #1673

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

sjbcastro
Copy link

@sjbcastro sjbcastro commented Jun 9, 2020

Fixes #1623

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm run test-all
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-reviewers for help, in a comment below
  • Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

@welcome
Copy link

welcome bot commented Jun 9, 2020

Thanks for opening this pull request!
There may be some errors, but don't worry! We're here to help! 👍🎉😄

@sjbcastro sjbcastro changed the title Stopped the step options being set to its parent to avoid recursion. Fixed the step options being set in an infinite loop Jun 9, 2020
@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #1673 (d81027d) into main (853e719) will decrease coverage by 1.79%.
The diff coverage is 46.48%.

❗ Current head d81027d differs from pull request most recent head a0e2714. Consider uploading reports for the commit a0e2714 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1673      +/-   ##
==========================================
- Coverage   66.67%   64.88%   -1.80%     
==========================================
  Files         130      135       +5     
  Lines        2686     2828     +142     
  Branches      433      458      +25     
==========================================
+ Hits         1791     1835      +44     
- Misses        895      993      +98     
Impacted Files Coverage Δ
src/InsertStep.js 79.41% <ø> (ø)
src/Modules.js 100.00% <ø> (ø)
src/modules/EdgeDetect/Module.js 100.00% <ø> (ø)
src/modules/WebglDistort/Module.js 2.29% <0.00%> (ø)
src/modules/ColorHalftone/Module.js 3.63% <3.63%> (ø)
src/modules/Shadow/Module.js 44.30% <44.30%> (ø)
src/modules/Overlay/Module.js 93.18% <94.11%> (-1.69%) ⬇️
src/modules/ColorHalftone/index.js 100.00% <100.00%> (ø)
src/modules/Colorbar/Module.js 100.00% <100.00%> (ø)
src/modules/Crop/Module.js 88.88% <100.00%> (ø)
... and 9 more

@sjbcastro
Copy link
Author

Hi @harshkhandeparkar just realised this pull request is still open. Are there any changes I need to make or can it be merged?

@harshkhandeparkar
Copy link
Member

I am just a reviewer here (and oops I forgot to approve 😝) so I can't merge it. @jywarren is the maintainer but I haven't seen him merge PRs in a while ¯_(ツ)_/¯

@gitpod-io
Copy link

gitpod-io bot commented Oct 4, 2020

@jywarren
Copy link
Member

jywarren commented Oct 4, 2020

Hi all! Thanks, and sorry for the quietness recently. Am trying to review more regularly.

Looks like a couple test jobs failed... can you take a look at what's going wrong there? If you paste the output into a comment, we can debug together. Thanks!

image

@harshkhandeparkar harshkhandeparkar requested a review from a team as a code owner October 15, 2020 20:40
@jywarren jywarren added this to the v3.7.0 milestone Oct 28, 2020
@harshkhandeparkar harshkhandeparkar removed this from the v3.7.0 milestone Nov 2, 2020
@gitpod-io
Copy link

gitpod-io bot commented Jul 27, 2021

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.

Problem with step object
4 participants