Skip to content

skpkg: migrate documentation, README, and public static files #203

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

Closed
wants to merge 9 commits into from

Conversation

zmx27
Copy link
Contributor

@zmx27 zmx27 commented Jun 11, 2025

No description provided.

@zmx27 zmx27 marked this pull request as draft June 11, 2025 17:46
Copy link

codecov bot commented Jun 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.23%. Comparing base (fc5bac1) to head (c83de82).
Report is 1 commits behind head on migration.

Additional details and impacted files
@@            Coverage Diff             @@
##           migration     #203   +/-   ##
==========================================
  Coverage      97.23%   97.23%           
==========================================
  Files             20       20           
  Lines            903      903           
==========================================
  Hits             878      878           
  Misses            25       25           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

I know htis is a draft but I left some comments. I expect that many of the "deleted" files will reappear as you move them over. Thanks for putting the PR up early as a draft!

AUTHORS.rst Outdated

Billinge Group and community contributors.
Sangjoon Lee, Simon Billinge, Billinge Group members
Copy link
Contributor

Choose a reason for hiding this comment

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

This should stay as it was.

CHANGELOG.rst Outdated
=============

.. current developments

Copy link
Contributor

Choose a reason for hiding this comment

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

we need all these old changelogs.

@@ -67,7 +67,7 @@ Enforcement

Instances of abusive, harassing, or otherwise unacceptable behavior may be
reported to the community leaders responsible for enforcement at
[email protected]. All complaints will be reviewed and investigated promptly and fairly.
[email protected]. All complaints will be reviewed and investigated promptly and fairly.
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be my columbia email if possible. If this change is coming from skpkg, could we put in an issue to change it there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change comes from the original pyproject.toml file, which listed your non-Columbia email. I can change this back

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha. Yes, I would like this changed to my Columbia email in this case, though it was a good choice to have the instinct to change it the way you did....

@zmx27
Copy link
Contributor Author

zmx27 commented Jun 11, 2025

@sbillinge when trying to migrate the doc/manual directory, I found that certain files were too large (exceeds 500KB) and pre-commit wouldn't pass. How should I go about doing it?

@sbillinge
Copy link
Contributor

@zmx27 I often see merge commits in your commit history. These should appear very rarely or never. Why are they happening?

@zmx27
Copy link
Contributor Author

zmx27 commented Jun 12, 2025

@sbillinge Sometimes it's because I forget to run git pull upstream after a previously open PR is merged, but in a lot of the other times I feel that it's due to the CI making changes to the code on GitHub, and my local repository cannot keep up. My pre-commit hooks are on, and everything should have already passed pre-commit when I make a commit, so I don't know what's going on. On a side note, how should I migrate the doc/manual folder, if at all? Some of the files seem to be too large, thus failing pre-commit. I think that this is my last step before making final checks.

@zmx27 zmx27 marked this pull request as ready for review June 12, 2025 06:46
@sbillinge
Copy link
Contributor

@sbillinge Sometimes it's because I forget to run git pull upstream after a previously open PR is merged, but in a lot of the other times I feel that it's due to the CI making changes to the code on GitHub, and my local repository cannot keep up. My pre-commit hooks are on, and everything should have already passed pre-commit when I make a commit, so I don't know what's going on. On a side note, how should I migrate the doc/manual folder, if at all? Some of the files seem to be too large, thus failing pre-commit. I think that this is my last step before making final checks.

so this not the correct workflow. First always sync upstream and build your new branch from the base. But if you did that, you don't synchronize your local with upstream except on rare occasions. which would be discussed in the PR convo. If things have diverged with upstream and there is a conflict, we would like to understand why before just simply merging.

For the manual migration, we do want to move it over. Make a PR moving everything that is not too bg and we will merge that. Then describe in the PR which files are failing "too large" check and we can discuss.

@zmx27
Copy link
Contributor Author

zmx27 commented Jun 12, 2025

I think I will just redo this branch instead. I will make a new PR when I'm finished.

@zmx27 zmx27 closed this Jun 12, 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.

2 participants