Skip to content

Conversation

@stevenhua0320
Copy link
Contributor

@zmx27 Ready for review, basically all operations the same as #11

  • Remove all unused modules
  • qt4 to qt for py2 to py3

Copy link
Collaborator

@zmx27 zmx27 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@stevenhua0320
Copy link
Contributor Author

@sbillinge Ready for review as reviewed by Zhiming

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @stevenhua0320

Don't go back and change anything, but please use group standard commit messages and also for the PR title. These are prepended with the nature of the change followed by a colon and then a description that helps the reviewer keep track of the task as best as possible. You can find these described in the scikit-package documentation.

You have made a functional change ("qt") so please can you report on how this affected the code passing tests or how, in other words, you checkd that this worked? Alternatively, how you learned that this change was needed.

@stevenhua0320
Copy link
Contributor Author

@sbillinge the functional change qt is needed as qt4 is now an outdated version with no ongoing support and qt is by dedault the current framework. Basically it changes no functionality.
As for the new standard of commit and PR title, I would refer to Scikit-package documentation and follow with that in the next PR.

@stevenhua0320 stevenhua0320 changed the title Remove unused module and other flake8 check chore: Remove unused module and other flake8 check Sep 26, 2025
@sbillinge
Copy link
Contributor

Thanks @stevenhua0320 that is helpful. Justifying where you got the info about the qt change is also helpful in general...for example copy-pasting the source of the information can help reviewers know the quality of that information without increasing time overhead on you.

@sbillinge sbillinge merged commit 3d52757 into diffpy:migration Sep 26, 2025
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