Skip to content

Add changelog (NEWS.md), associated pkgdown styling #222

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

Merged
merged 4 commits into from
Oct 21, 2022

Conversation

brookslogan
Copy link
Contributor

@brookslogan brookslogan commented Aug 30, 2022

Part of #177.

Decisions to be made:

  • Is this version breakdown appropriate?
  • Is this changelog format appropriate? What changes do we want to make now? And moving forward?
  • Should main continue to be a development branch? If so, we can just merge this into main. If not, we should PR the latest release into main so it gets the right version number[, or] just bump the version number in this PR [and release/tag it] (--> 0.5.1).

Dan & Dmitry, your input would be very helpful here.

I've prepared some branches like 0.1.0_prep that we could then tag as 0.1.0 if we're happy with the retro version numbering. We should probably update the changelog inside of PRs moving forward.

(Note: we are trying to follow semver, but the .9999 thing wouldn't follow. It's to mark it as a dev version, not a release version. Seen in other R packages. The idea is to, at release time, look at the changes/changelog and decide what the semver release version number should be. Murky on what people actually recommend here.)

@brookslogan brookslogan requested a review from dshemetov August 30, 2022 14:36
@brookslogan brookslogan requested a review from dajmcdon as a code owner August 30, 2022 14:36
@dajmcdon
Copy link
Contributor

On the version breakdown:

  • I think more than appropriate.
  • Appropriate. Though, I'm not clear on the distinction between Improvements and Cleanup. What about "Improvements and other changes" all together?
  • (We should address this in epipredict too). Is the thinking to tag the release whenever x.x.x goes to x.x.(x+1)? If so, then, assuming the package is sufficiently stable, I think it's fine to have main==dev. The biggest reason to keep them separate is stability. But if there's a clear version numbering, one can always go back to the release.

I'm not sure I understand the last parenthetical. If we're going to main==dev with releases on the change in third digit, then we'll need the fourth digit for pre-release dev. Right? Maybe I'm missing something

@brookslogan
Copy link
Contributor Author

  • On 1: do you mean version breakdown is okay, or we should have fewer versions?
  • On 2: improvements vs. cleanup: improvements "impact" end-user; cleanup doesn't "impact" end-user. For a fuzzy definition of "impact"; e.g., I guess dealing with dependencies being kicked off CRAN does impact user installation. This could be clearer, but I'm not sure exactly how to draw the line/lines. Any ideas?
  • On 3&4: yes, seems that tagging each x.x.(x+1) update as a release and using main==dev and 4th digit for post-release development works. An alternative might be a dev branch with/without a 4th digit. There may not be much of a distinction between the two approaches if we are marking the dev branch as default PR destination but it ALSO as a consequence gets marked as the default install_github branch.

@brookslogan
Copy link
Contributor Author

The main distinction is if epiprocess development is pushing directly to whatever branch/version epipredict is relying on.

  • Same --> more breakage, faster feedback, trouble for users&pkgdown trying to test out during these times
  • Different --> no spontaneous breakage, but need to periodically sync epipredict with later epiprocess versions and address breakage then

For the still-still-pending PRs and issues with breaking changes that we're trying to get through soon in epiprocess, we should at least partially follow approach 2. But once they're all ready on some non-default branch, we'll be faced with a similar decision:

  • Point epipredict to a stable branch/tag, then pull the breaking changes into epiprocess main. Periodically update epipredict's dependency on epiprocess.
  • Create a non-default dev branch in epiprocess, pull the breaking changes into epiprocess main. Periodically update epipredict's dependency on epiprocess.
  • Pull the breaking changes into epiprocess default branch (main or dev). epipredict will break. Fix epipredict. Repeat periodically.

@dshemetov
Copy link
Contributor

Looking at the mature Python package more_itertools for inspiration, they use:

  • New itertools
  • Change to existing itertools
  • Other changes
  • Bug fixes

So they choose to emphasize when a central piece is either added or updated. The "improvement" distinction is decided on each line when describing the change e.g.

The implementation for set_partitions() was improved. (thanks to jferard)

We could do something similar for the core functionality like epi_df and epi_archive.

@dshemetov
Copy link
Contributor

dshemetov commented Aug 31, 2022

I'm not familiar enough with the details of this package to comment on past version attribution. So I'll only comment on main vs dev as development branch.

My preference for having a dev branch is to have main always point to the latest stable release. This way someone can just specify this repo, if they want to install the latest stable version. Unfortunately, this conflicts with making dev the default branch that PRs are merged against, which is annoying, but something we could live with.

I'm not entirely sure I understand the .9999 thing, but if it's just a placeholder until we agree on a release version, sounds good.

And if I'm following the epipredict sync thread (which I'm not really sure that I am), I think that not having to pin epipredict to epiprocess versions every time is a benefit of having a separate main and dev branch -- epipredict can just point to main most of the time and if there's something going on in epiprocess that is temporarily incompatible, then we can just pin epipredict to a release until they become compatible again.

Copy link
Contributor

@dshemetov dshemetov left a comment

Choose a reason for hiding this comment

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

See my comment above.

`pkgdown` seems to skip material preceding the first version number heading, as
well as top-level headings that don't follow a certain format. Move the intro
material to somewhere near the top where it won't be skipped; we'll need to move
it again each time we release a version.
@brookslogan
Copy link
Contributor Author

brookslogan commented Oct 21, 2022

Thanks for the comments! I'm going ahead and merging this [so we can start adding changes to the changelog as part of other PRs]. Some improvements are deferred to later.

@brookslogan brookslogan merged commit 94d5ca5 into main Oct 21, 2022
@dshemetov dshemetov deleted the lcb/add-changelog branch August 23, 2023 19:26
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.

4 participants