Closed biswajitSM closed 7 years ago
@biswajitSM, contributions to expand the range of supported file format are certainly well accepted.
However, the way you generated this PR has some problems. It seems you based your work on a much older version of phconvert and the PR is trying to revert a lot of recents modifications. If you look at the "File changes" pane you can see it yourself. For example, there are modifications to hdf5.py, but I think these are only reverting recent changes. Also, you first delete an then re-add a notebook, this creates big diff that's are hard to review.
In general the scope of a PR should be with a clearly defined scope (as it is in your case), and the modifications should be only related to that topic, to make it easy review an testing. I give you some tips, let me know if you have any problems.
You should base your work on the most current master (or at least a recent commit). Briefly:
checkout the most recent version of phconvert from master. If you forked the repository a while ago you need to sync your fork with the latest version here first.
create a new branch and switch to it, for example:
git checkout -b pt3t3rsupport
make your modifications, only changing code related to this PR
commit and push the new branch to your own fork. You can split your work in 2 commits, one adding the new functions and one adding the new notebook. But only one commit would work too. One important thing. When you commit a notebook, please commit the version with no output. The notebook will be automatically executed by TraviCI and AppeVeyor whenever you submit a PR.
You open a new PR from the branch in your fork to upstream (here)
I don't know what went wrong in your case, but try to follows these steps to make a clean PR that I can review. You can either try to overwrite this PR or open a new one, whatever it is easier.
More info:
I work with pt3 and t3r file. So I added few reader-functions to read those files. To stay up-to-date with the phconvert-master branch, I thought may be I can PR. Thanks