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

Refactoring Branch #17

Merged
merged 24 commits into from
Apr 2, 2018
Merged

Refactoring Branch #17

merged 24 commits into from
Apr 2, 2018

Conversation

AndreG-P
Copy link
Member

@AndreG-P AndreG-P commented Feb 22, 2018

@physikerwelt @vstange
I started to refactor MathMLTools as mentioned in #13

This branch contains now:

  • MathMLTools (in mathml-core now)
  • MathMLConverters (in mathml-converters now)
  • MathMLSim (in mathml-similarity now)

All tests except one (see #16) are finished successful on my machine (we will see what travis will show us...).

I don't know exactly why... Just before I start to refactor the project, I merged the WIP branch from Moritz #8 to master. I made some fixes but it looked like it worked. I merged it because I wanted the latest version in the refactored project (I presumed to solve merge conflicts after the refactoring will be a nightmare).

Besides this, I will change the naming a little bit further.

However, I'm more happy with the new structure. I need to setup the auto deployment system again, since it is a multi-module project now (see #15). Any suggestions for further improvements would be nice.

PS: I needed to merge the travis config also. We should definitely find another solution for this travis hack with mathoid. See gipplab/mathosphere#108

@AndreG-P AndreG-P self-assigned this Feb 22, 2018
@AndreG-P
Copy link
Member Author

Okey... Travis failed. It looks like the mathoid version is not running in travis. When I don't start Mathoid on my local machine, I got the same errors in the tests. When I start Mathoid, the tests finish successfully. @vstange Any idea what is missing in the travis config for MathMLConverters?

@physikerwelt
Copy link
Member

@AndreG-P you spend soo much time to write the logs. Now you also have to read them
https://travis-ci.org/ag-gipp/MathMLTools/builds/344730018#L2477
;-)

@vstange
Copy link
Member

vstange commented Feb 22, 2018

mathoid didn't start see travis output on:
Error while reading config file: Error: ENOENT: no such file or directory, open '/home/travis/build/ag-gipp/MathMLTools/config.dev.yaml'

MathMLConverters has a custom config for mathoid: https://github.com/ag-gipp/MathMLConverters/blob/master/config.dev.yaml

by adding junit-jupiter-engine to maven surefire.
The config for mathoid is only necessary because of travis installations. So I moved it to the .travis folder.
Also, an older version of mathoid is used (from https://git@github.com/physikerwelt/mathoid-server).
We should fix this kind of tests!
@coveralls
Copy link

coveralls commented Feb 23, 2018

Coverage Status

Coverage increased (+4.5%) to 49.855% when pulling 50bca81 on refactor into 8bc5867 on master.

Splitted the original MathMLTools classes to mathml-core and mathml-utils.
@AndreG-P AndreG-P mentioned this pull request Mar 15, 2018
Closed
@AndreG-P AndreG-P merged commit 50bca81 into master Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants