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

Remove NEWS functionality #12

Open
wants to merge 6 commits into
base: devel
Choose a base branch
from
Open

Remove NEWS functionality #12

wants to merge 6 commits into from

Conversation

LiNk-NY
Copy link
Contributor

@LiNk-NY LiNk-NY commented Feb 10, 2023

Hi Lori,

All NEWS related functions are going to ReleaseLaunch.
I've updated the documentation and the NEWS file to reflect this change.

With approval, I can merge and update upstream.

Thanks!
-MR

@LiNk-NY LiNk-NY requested a review from lshep February 10, 2023 22:10
@LiNk-NY LiNk-NY self-assigned this Feb 10, 2023
@lshep
Copy link
Contributor

lshep commented Feb 13, 2023

@jwokaty @hpages The BBS extracts and puts the NEWS files on master for each package that has one -- not sure where this happens but does this need to be updated before or in coordination with the transition of these functions? if these functions are no longer in biocViews that will likely also mean a manual install of ReleaseLaunch on the daily builders

@lshep lshep requested review from hpages and jwokaty February 13, 2023 16:32
@hpages
Copy link
Contributor

hpages commented Feb 14, 2023

BBS uses the extractNEWS() function defined in biocViews/R/repository.R during the daily propagation (extractNEWS() is called by prepareRepos.sh). Unless this function is used at release time as part of the process of compiling the NEWS then it would be better to leave it in biocViews.

More generally speaking my impression is that everything that is in biocViews/R/repository.R is for the exclusive use of BBS's propagation scripts so should not be touched.

@LiNk-NY
Copy link
Contributor Author

LiNk-NY commented Feb 14, 2023

It looks like extractNEWS() is run whenever prepareRepos.sh is run (nearly daily?) to populate the NEWS link in the landing page. For release packages, it is likely unnecessary as the NEWS file does not change often. For devel, some maintainers (including myself) update the NEWS near the end of the devel cycle. Others update as they make changes to the package.
I can leave the functionality as-is in biocViews and move the rest to ReleaseLaunch. This may lead to split functionality in both packages though...

@hpages
Copy link
Contributor

hpages commented Feb 14, 2023

This may lead to split functionality in both packages though...

Right but it's not that one set of functions needs to know about the other one, AFAICT they don't share any code. Also the context in which they are used is very different: as part of the automated daily builds for one, only once every 6 months and manually for the other one. So keeping them separated might actually be a good thing.

BTW it seems that getNEWSFromFile() would also need to stay in biocViews.

Thanks!

@LiNk-NY
Copy link
Contributor Author

LiNk-NY commented Feb 17, 2023

It looks like it is ready to go. Let me know if I missed something.
Squash and merge is preferred.
-Marcel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

3 participants