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

Retain IDF order and comments #261

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

hnagda
Copy link

@hnagda hnagda commented Sep 9, 2019

This addresses issue #260

The code was written about 3 months ago and it just works. There is a lot of room for improvement in the way its written (especially in withcomment() function). I would appreciate any comments or ideas to do things in a better way.

Added the functions to save the IDF in original order and retain comments
Added the saveinorder() function to retain the original order of IDF files and all the comments.
@santoshphilip santoshphilip self-assigned this Sep 10, 2019
@santoshphilip
Copy link
Owner

santoshphilip commented Sep 10, 2019

@hnagda , I am not seeing any unit tests.
Can you make some unit tests for your changes.
The tests should show that the save is saving the objects in the order expected. Look at other unit tests in folder ./eppy/tests/ to get a feel for how the tests are done. Ask if you need help.

In principle, if you have good unit tests and all existing tests and new tests pass, the pull request should be accepted. even without looking at your code :-)

In reality, we will look at your code and comment if needed.

@lymereJ
Copy link
Contributor

lymereJ commented May 30, 2023

@hnagda - any updates?

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.

3 participants