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

Rename files for meta examples #4055

Merged

Conversation

dgkim5360
Copy link
Contributor

Referring #4050

@lisitsyn
Copy link
Member

@dgkim5360 thanks for the PR and welcome!

@karlnapf I think this one is for you

@karlnapf
Copy link
Member

karlnapf commented Jan 5, 2018

Great stuff, thanks for the patch!!!
One thing is that the data submodule needs to be updated first as I think all integration tests will fail otherwise. Or am I wrong?
Check the developing and examples readme on how to run those tests

@karlnapf
Copy link
Member

karlnapf commented Jan 5, 2018

I.e. each meta example generates output of the same filename as itself.
Then the output is compared across the different languages and the c++ version.
Now if you rename the files, our build simply doesnt run the tests anymore (check output of the c++ build)

@karlnapf
Copy link
Member

karlnapf commented Jan 5, 2018

See here https://travis-ci.org/shogun-toolbox/shogun/jobs/322523257#L4853

you can see that only the tests for the not-renamed files were executed.
You will need to rename the test files as well, send a PR to the data submodule, and then update the submodule version in this PR here. Then the tests will be executed and we can merge this.
Thanks so much for the effort!

@dgkim5360
Copy link
Contributor Author

I see that's why the number of tests decreased when I renamed the files. I will check the READMEs and tests again, and also the data submodule. I appreciate your guides.

@dgkim5360
Copy link
Contributor Author

After checking for renaming in the data module to recover the test cases, I have sent a PR to shogun/shogun-data. Is this enough, or anything else to do for this PR? Please let me know :)

@dgkim5360 dgkim5360 force-pushed the feature/meta-examples-filenames branch from 6451016 to 8aaac8e Compare January 7, 2018 05:35
@dgkim5360
Copy link
Contributor Author

I just recognized I have to update this PR for triggering CI (updating submodule version as you said), but I felt something wrong with the git working flow. Excuse me for taking some time to figure out for clean commits...

@dgkim5360
Copy link
Contributor Author

I am not sure the job is done well. Please tell me if I need to adjust !

@karlnapf
Copy link
Member

Looks good to me! Thanks a lot for the cleanup

@karlnapf karlnapf merged commit 5d1e93d into shogun-toolbox:develop Jan 11, 2018
@dgkim5360 dgkim5360 deleted the feature/meta-examples-filenames branch January 15, 2018 01:00
@dgkim5360
Copy link
Contributor Author

This was my first contribution for the open source project. Grateful for such opportunity and experience.

@karlnapf
Copy link
Member

Well done on that! :)
Feel free to do another one ... always welcome!

ktiefe pushed a commit to ktiefe/shogun that referenced this pull request Jul 30, 2019
* Rename files for meta examples
* Update the data submodule version
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.

None yet

3 participants