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.

Sign up for GitHub

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

Jump to bottom

Map types to html types #754

Merged
merged 12 commits into from
Mar 4, 2019
Merged

Map types to html types #754

merged 12 commits into from
Mar 4, 2019

Conversation

harshkhandeparkar
Copy link
Member

@harshkhandeparkar harshkhandeparkar commented Feb 9, 2019 •
edited by grvsachdeva
Loading

Fixes #570

@jywarren I think this might need another rebase. Can you please do that? Also check if all of the changes are correct(review). Thanks!

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/reviewers and @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!

@harshkhandeparkar
Copy link
Member Author

@jywarren The commit history and the changes look a bit crazy because I might have merged something unnecessary. Please check if everything is fine and please rebase if necessary. Thanks!

@jywarren
Copy link
Member

jywarren commented Feb 9, 2019 via email

@harshkhandeparkar
Copy link
Member Author

@jywarren No problem. I understand you are busy. I'm not at home I will either do that tonight(IST, a few hours) or tommorow. Btw does it need a rebase anyways??

@harshkhandeparkar
Copy link
Member Author

Are the changes good? I don't think it needs a rebase

@harshkhandeparkar
Copy link
Member Author

@jywarren there are no merge conflicts

jywarren
jywarren requested changes Feb 10, 2019
View reviewed changes
dist/image-sequencer.js Outdated
if (returnInverted) {
inverted.set(x, y, !(lum <= threshold));
}
for (var x = 0; x < REGION_SIZE; x++) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just worried about this part, can you try rebuilding dist files to be sure we're not reverting something important?

harshkhandeparkar
harshkhandeparkar commented Feb 10, 2019
View reviewed changes
Copy link
Member Author

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need help

package.json Outdated Show resolved Hide resolved
@harshkhandeparkar
Copy link
Member Author

Also is there anything that is to be added to the docs?

@grvsachdeva
Copy link
Member

Suggestion: Updating the PR checklist and adding a label to the PR as per status help reviewers in filtering out, so it's a good practice.

HarshKhandeparkar and others added 11 commits March 4, 2019 10:56
@jywarren
fix insert step button
9fe1290
@jywarren
add dist
d6ca99d
@jywarren
add new function
fb635b8
@jywarren
remove console logs
c9f58ad
@jywarren
refactor
d199856
@vibhorgupta-gh @jywarren
Setup UI testing (#720)
319b06c 
* default sequencer ui test

* default step ui test suite

* intermediate step ui test

* preview ui test suite

* url methods test suite

* add set url params method test suite

* argument call tests

* test directory refactor

* travis fix
@vibhorgupta-gh @jywarren
CLI code refactor (#665)
d7bbad6 
* CLI refactor

* es6 rollback

* Travis fix

* syntax fix

* clustered require statements

* travis debug

* travis debug
@gfting @jywarren
Added line that selects the "More modules..." after adding a step. (#713
e120239 
)

* Added line making module selection correct after adding a step

* Added line with appropriate module selection
@vibhorgupta-gh @jywarren
Add: Module tests (#748)
7b64d56 
* WIP

* module testing harness

* adjustments
@jywarren
Update package-lock.json
17ce2f0
@dependabot @jywarren
Bump jsqr from 1.1.1 to 1.2.0 (#749)
3a63378 
Bumps [jsqr](https://github.com/cozmo/jsQR) from 1.1.1 to 1.2.0.
- [Release notes](https://github.com/cozmo/jsQR/releases)
- [Commits](https://github.com/cozmo/jsQR/commits)

Signed-off-by: dependabot[bot] <support@dependabot.com>
@jywarren
Copy link
Member

jywarren commented Mar 4, 2019

Oh my gosh that was the worst rebase EVER! I think i got it though...

@jywarren jywarren merged commit 1e08d7c into publiclab:main Mar 4, 2019
@jywarren
Copy link
Member

jywarren commented Mar 4, 2019

🎉 🎉 🎉 wow that was a tough merge! But completed!!!! Thanks, Harsh!

@harshkhandeparkar harshkhandeparkar deleted the map-types-to-htmlTypes branch June 7, 2019 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sashadev-sky sashadev-sky sashadev-sky left review comments

@jywarren jywarren jywarren requested changes

Assignees
No one assigned
Labels
enhancement review-needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@harshkhandeparkar @jywarren @grvsachdeva @sashadev-sky @vibhorgupta-gh @gfting