Skip to content
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

Release roam 0.1.0 #2

Open
13 of 28 tasks
mitchelloharawild opened this issue Mar 22, 2023 · 8 comments
Open
13 of 28 tasks

Release roam 0.1.0 #2

mitchelloharawild opened this issue Mar 22, 2023 · 8 comments
Assignees

Comments

@mitchelloharawild
Copy link
Collaborator

mitchelloharawild commented Mar 22, 2023

First release:

Prepare for release:

  • git pull
  • Check if any deprecation processes should be advanced, as described in Gradual deprecation
  • devtools::build_readme()
  • urlchecker::url_check()
  • devtools::check(remote = TRUE, manual = TRUE)
  • devtools::check_win_devel()
  • rhub::check_for_cran()
  • git push
  • Draft blog post

Submit to CRAN:

  • usethis::use_version('minor')
  • devtools::submit_cran()
  • Approve email

Wait for CRAN...

  • Accepted 🎉
  • git push
  • usethis::use_github_release()
  • usethis::use_dev_version()
  • usethis::use_news_md()
  • git push
  • Finish blog post
  • Tweet
  • Add link to blog post in pkgdown news menu
@mitchelloharawild
Copy link
Collaborator Author

TODO:

  • Improve the README with additional details, rather than the default of 'the goal of roam is to...'
  • Add more documentation for creating a roam dataset, possibly in a vignette or at least adding more information in the README (e.g. about versioning)
  • Add more details in NEWS relating to what this initial release allows you to do
  • The skip for tests should exist in the tests, not code:

    roam/R/roam.R

    Line 73 in 59818b6

    if(!is.na(Sys.getenv("R_TESTS", unset = NA))) return(invisible(NULL))
  • Bump package version for release
  • A few design improvements. For example, roam_version() shouldn't have two purposes / functionalities. Instead create a different function for setting the version, like roam_set_version() to be used in obtainer functions.
  • Probably some improvements to versioning, like attaching it to the same cached roam object? I'm not sure how this currently works since there weren't examples I could find easily.

There's some code design improvements possible too, a lot of the flags aren't needed but they can be improved in future versions. I think it's most important to have a good design for the first release, where future improvements can be made with minimal breaking changes for users.

@mitchelloharawild
Copy link
Collaborator Author

Any updates on CRAN release @FinYang ?

@FinYang
Copy link
Owner

FinYang commented Jan 16, 2025

No updates 😢. Are we going with the current implementation of versioning or do we want to do some rewrites? If going with the current one and only adding vignettes in later versions, I can get it up there soon. Otherwise...

@mitchelloharawild
Copy link
Collaborator Author

The versioning can be backwards compatible can't it? If so - then current version to cran is fine.

@FinYang
Copy link
Owner

FinYang commented Jan 16, 2025

Right now we require the package writer to 1. define the obtainer function with an argument called version to accept version number from the user, and 2. call the roam_set_version in the obtainer function to set the version. If we want to keep this behaviour, then it's backwards compatible.

@mitchelloharawild
Copy link
Collaborator Author

Not sure about the need for roam_set_version(), presumably for the default version ("latest")?
I think that can be refactored into an additional method for the obtainer like "get_version()" which has a default method returning <tag>, e.g. "latest" -> "latest", "4.3.0" -> "4.3.0" which should work for most cases. I think it's only needed if the dataset is strictly versioned which it often won't be.

I can look into it further.

@FinYang
Copy link
Owner

FinYang commented Jan 16, 2025

The roam_set_version() is just so that we know what version it is to cache.

The pkg writer doesn't need to use it. If not used, it will save the version as NA. I didn't want to save version as "latest" by default, because it is only "latest"when the user cache the data the first time. Two months later, when the user wants to check the version of the cached data, it doesn't make sense to return "latest", because it won't be the latest in the remote source. If they see "latest", they might think it is up to date, when it is not.

This also makes sense when the dataset is not versioned. If the dataset is not versioned, "latest" can imply there are multiple versions, when there is one only.

If the dataset is strictly versioned, the pkg writer can use roam_set_version() to tell us the version of the cached data, so that we can record it locally. The pkg writer can get this version number however they want inside the obtainer function, query a remote server, etc.
This is backwards compatible, when we later want to extend roam to cache multiple versions at the same time.

@mitchelloharawild
Copy link
Collaborator Author

Fair enough, although I use "latest" in the container sense - where it is latest with a date stamp (i.e. latest last updated at that time). NA is also fine.

I think roam_set_version() should be outside of the obtainer; a separate method with suitable (identity()) default. That way the obtainer function doesn't have side-effects.

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

No branches or pull requests

2 participants