Skip to content

DM-7165: A new class, PhaseFoldedLightCurve, to add a phase column in… #169

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

Merged
merged 2 commits into from
Sep 12, 2016

Conversation

ymeiymei
Copy link
Contributor

@ymeiymei ymeiymei commented Sep 9, 2016

@ejoliet Please review and test.

…to the input data group.

Also the test harness class and the testing data.

private static final String TEST_ROOT = "test"+ File.separatorChar;
private static String inFileName = "/hydra/cm/firefly/src/firefly/test/edu/caltech/ipac/firefly/server/query/AllWISE-MEP-m82-2targets-10arsecs-oneTarget.tbl";
//private static String inFileFullName = TEST_ROOT + inFileName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, make the test to read the file relative to the test folder. This file path is absolute and note necessary match

@ejoliet
Copy link
Contributor

ejoliet commented Sep 10, 2016

Good!
Couple of cleanup though to be fixed:

  • Test doesn't work because the test file is defined with its absolute path. Please, change to relative path.
  • Remove 'System.out' or change to use logger.

@ejoliet
Copy link
Contributor

ejoliet commented Sep 12, 2016

Good. Review done.

@ymeiymei ymeiymei merged commit c44ec78 into dev Sep 12, 2016
@ymeiymei ymeiymei deleted the DM-7165-PhasefoldedLC branch September 12, 2016 21:17
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.

2 participants