-
Notifications
You must be signed in to change notification settings - Fork 33
enh(policy): Add discussion of package length #358
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these changes were made by an autoformatter? Either way, everything in here looks good and should probably add markdown formatting checks to CI.
about/package-scope.md
Outdated
|
|
||
| ### Package size and scholarly effort | ||
|
|
||
| PyOpenSci reviews packages that represent substantial scholarly effort and provide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| PyOpenSci reviews packages that represent substantial scholarly effort and provide | |
| pyOpenSci reviews packages that represent substantial scholarly effort and provide |
about/package-scope.md
Outdated
| The final decision on whether a package represents sufficient scholarly effort | ||
| rests with the editor and will be made on a case-by-case basis. | ||
|
|
||
| ```{note} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ```{note} | |
| :::{note} |
just for consistency as we use colon fences throughout (works either way tho!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lwasser I think this was changed on save with the autoformatter I was using. Is there a way to have colon fences enforced through autoformatting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eliotwrobson what are you using here on save? I think either we need a consistent linter for markdown or turn off linting on save-- otherwise this will keep happening every commit/PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yeelauren looking at it again, I think that the AI assistant I was using did this. I can change back without having to mess with my settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yeelauren raises a good point. Could we create an .editorconfig file? I set up pre-commit locally, (there are .precommit config files in most of our repos here) so when I commit, it will format the files for me. If you prefer format on save, we could create a .editorconfig file too and just use that for those who don't like pre-commit. I know some really dislike it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having an editorconfig is a good idea (I personally like format-on-save more than on commit). We should make a separate issue + PR for that though (I think this one is ready to go anyway).
about/package-scope.md
Outdated
| Our more flexible approach allows us to support useful scientific packages | ||
| that may be outside JOSS scope while maintaining our partnership for | ||
| packages that meet both organizations' criteria. | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ``` | |
| ::: |
about/package-scope.md
Outdated
| ```{note} | ||
| **Relationship to JOSS requirements:** | ||
| The Journal of Open Source Software (JOSS) requires packages to have at least |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also link to JOSS policies here for "validation" .
our-process/policies.md
Outdated
| standards. This approach reduces the burden of a full review while ensuring the quality of the package | ||
| reflects its most recent version. | ||
|
|
||
| ```{note} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ```{note} | |
| :::{note} |
our-process/policies.md
Outdated
| but does not meet JOSS requirements (e.g., minimum lines of code), it will not be | ||
| eligible for JOSS fast-track review. See our [package scope guidelines](../about/package-scope.html#package-size-and-scholarly-effort) | ||
| for more details. | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ``` | |
| ::: |
lwasser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes here!!
| minimum line count, we expect packages to demonstrate sufficient complexity and | ||
| functionality to warrant a comprehensive peer review. | ||
|
|
||
| **General expectations:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great additions
|
@lwasser In the latest push, I believe all of the comments you raised have been addressed. |
lwasser
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eliotwrobson this looks good to me. I think @yeelauren requested changes, but I also don't see what those changes are, so maybe you've completed them!! I'm happy with merging this. I'll open a tiny pr to fix the lingering broken target/link. I don't understand why those are popping up in this pr but they are!
| for an individual developer | ||
| - We discourage "salami slicing" - breaking up functionality into artificially | ||
| small packages solely to generate multiple submissions | ||
| - Packages with fewer than approximately 1,000 non-comment, non-whitespace lines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe note that unit tests are not counted toward the 1,000 lines minimum but are expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the link to package scope guidelines for clarity.
| package size requirements), packages must still meet JOSS's specific criteria to | ||
| be eligible for JOSS fast-track submission. If your package is accepted by pyOpenSci | ||
| but does not meet JOSS requirements (e.g., minimum lines of code), it will not be | ||
| eligible for JOSS fast-track review. See our [package scope guidelines](package-size-effort) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created this target in the other PR to avoid future broken links. I 'm realizing that we have three pr's here that need to reference each other. 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good to try and push to lower the issue / PR count so we can reduce the overhead of stuff like that (I make mistakes like that all the time too!)
Summarized discussion in #351 and should be appropriate for closing the issue. Written using gen ai for summarizing.