Skip to content
This repository has been archived by the owner on Aug 29, 2024. It is now read-only.

file reorg #57

Merged
merged 29 commits into from
Dec 26, 2016
Merged

file reorg #57

merged 29 commits into from
Dec 26, 2016

Conversation

evykassirer
Copy link
Contributor

@evykassirer evykassirer commented Dec 15, 2016

TODO:

  • move tests for stuff in the util folder into the util folder
  • add more stuff into util folder: remove parens, flatten, possibly more
  • move rearrange coeff into simplify basics or a top level tree search
  • break up simplifyBasics functions so each one is its own file and tested by a separate file
  • split simplifyExpression into simplify file and stepThrough file (don’t export step at all, use the first step from stepThrough for tests that test step)

... and anything else!

NOT IN THIS PR: moving removeParens, that should be it's own PR

@evykassirer evykassirer changed the base branch from master to ek_tree_search December 16, 2016 17:29
@evykassirer evykassirer changed the base branch from ek_tree_search to master December 16, 2016 17:40
@evykassirer evykassirer mentioned this pull request Dec 17, 2016
8 tasks
@evykassirer
Copy link
Contributor Author

I'm gonna work on this some more today probably


module.exports = {
expressionStepper: simplifyExpression.stepThrough,
// TODO: after deploying this and then the update to Athena.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this part is important to see

@@ -1,420 +0,0 @@
'use strict';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PolynomialTermOperations.js is gone now! :)

return NodeStatus.noChange(node);
}

function multiplyPolynomialTerms(node) { // TODO: do this for add too
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh I can take out this TODO

return NodeStatus.noChange(node);
}
}
// TODO(ael): make nthRoot a folder and sort out these functions better
Copy link
Contributor Author

Choose a reason for hiding this comment

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

left this for later, cause I wasn't sure what the best way to structure it was

const reduceMultiplicationByZero = require('./reduceMultiplicationByZero');
const reduceZeroDividedByAnything = require('./reduceZeroDividedByAnything');
const rearrangeCoefficient = require('./rearrangeCoefficient');
// TODO: move the rest of the functions into their own files too
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might make this an issue, it's not super important but would feel cleaner I think

Copy link
Contributor

@aelnaiem aelnaiem left a comment

Choose a reason for hiding this comment

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

😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍

@aelnaiem
Copy link
Contributor

Let's merge this :)

@evykassirer
Copy link
Contributor Author

k one sec gonna remove a stale comment and merge

@evykassirer evykassirer merged commit 42911af into master Dec 26, 2016
@evykassirer evykassirer deleted the ek_file_reorg branch January 27, 2017 15:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants