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

text-overlay in Node #1125

Merged
merged 5 commits into from
Jul 15, 2019
Merged

text-overlay in Node #1125

merged 5 commits into from
Jul 15, 2019

Conversation

aashna27
Copy link

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 test
  • 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

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

Thanks!

@aashna27
Copy link
Author

@harshkhandeparkar any idea on this?

@aashna27
Copy link
Author

aashna27 commented Jun 25, 2019

@tech4GT @Divy123

@tech4GT
Copy link
Member

tech4GT commented Jul 2, 2019

Looks like colorbar was affected somehow, that's the test which is failing. Try running it maybe you'll find the problem.

@codecov
Copy link

codecov bot commented Jul 13, 2019

Codecov Report

Merging #1125 into main will increase coverage by 0.11%.
The diff coverage is 37.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1125      +/-   ##
==========================================
+ Coverage   55.66%   55.78%   +0.11%     
==========================================
  Files         114      114              
  Lines        2357     2352       -5     
  Branches      364      363       -1     
==========================================
  Hits         1312     1312              
+ Misses       1045     1040       -5
Impacted Files Coverage Δ
src/modules/TextOverlay/Module.js 33.33% <37.5%> (+9.8%) ⬆️

@aashna27 aashna27 changed the title text-overlay puppeteer text-overlay in Node Jul 13, 2019
@aashna27
Copy link
Author

aashna27 commented Jul 13, 2019

@jywarren @publiclab/reviewers I have changed this and text-overlay now works for node. Please review. The reason this one wasn't working on node was that $ wasn't being recognized.

@aashna27 aashna27 requested review from harshkhandeparkar, tech4GT, Divy123, jywarren and harshithpabbati and removed request for tech4GT July 13, 2019 16:37
@Divy123
Copy link
Member

Divy123 commented Jul 14, 2019

Great finding @aashna27 and is there something to change in the docs?

Rest I think this is ready to be merged 🎉 🎉

@aashna27
Copy link
Author

Great finding @aashna27 and is there something to change in the docs?

Rest I think this is ready to be merged tada tada

Thanks! we dont have docs I ll add them! 🙈

@Divy123
Copy link
Member

Divy123 commented Jul 14, 2019

Thanks! we dont have docs I ll add them!

Yeah, thanks a lot!!

@jywarren jywarren merged commit 9464522 into publiclab:main Jul 15, 2019
@jywarren
Copy link
Member

Hooray!!! Great work!!!

@harshkhandeparkar
Copy link
Member

lol, I was reviewing this.

@jywarren
Copy link
Member

Sorry, I saw only docs required and 2 reviews (myself included!) so I went ahead! Sorry Harsh, let's try to coordinate better in the future, I'll keep a closer eye out. The pending reviewers are not easy to see from mobile view!

@harshkhandeparkar
Copy link
Member

It actually doesn't matter much. I was the one who delayed this.

@aashna27
Copy link
Author

Thanks a lot everyone for the quick merge! 🤗

jywarren pushed a commit that referenced this pull request Dec 16, 2019
* text-overlay in node

* text-overlay in node

* docs added
@harshkhandeparkar harshkhandeparkar added this to Done in v3.6.0 Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v3.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants