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

pruneRepo and archivePackages updated to work with binary packages. #79

Merged
merged 2 commits into from Mar 27, 2019
Merged

Conversation

cstepper
Copy link

Hi Dirk,

we are implementing drat in our company to supply a CRAN-like repo to our coworkers. As we are on Windows, we usually provide *.zip packages.
We modified the functions archivePackages and pruneRepo in order to work with binary packages.

Best, Christoph

@eddelbuettel
Copy link
Owner

Thanks -- though twelve modified functions is a lot for an unannounced PR. We prefer prior discussion.

But I'll take a look.

R/addRepo.R Show resolved Hide resolved
R/archivePackages.R Outdated Show resolved Hide resolved
R/archivePackages.R Show resolved Hide resolved
R/archivePackages.R Outdated Show resolved Hide resolved
R/archivePackages.R Outdated Show resolved Hide resolved
R/archivePackages.R Outdated Show resolved Hide resolved
R/insertPackage.R Show resolved Hide resolved
R/insertPackage.R Show resolved Hide resolved
R/archivePackages.R Show resolved Hide resolved
R/archivePackages.R Outdated Show resolved Hide resolved
@eddelbuettel
Copy link
Owner

Looks good!

@eddelbuettel
Copy link
Owner

@jangorecki Do you want to take another look?

@eddelbuettel eddelbuettel merged commit afbfe93 into eddelbuettel:master Mar 27, 2019
@jangorecki
Copy link
Contributor

Looks good, I would only advice to put formatting changes, like a=1 into a = 1 into new PR (or at least separate commit) so PR contains change of logic only. It is easier to follow as less lines are needed to be looked at.

@cstepper
Copy link
Author

Hi @eddelbuettel and @jangorecki , thanks for your very helpful comments (especially on how to do a PR). Looking forward to use the updated version here at work.

@eddelbuettel
Copy link
Owner

Yes, both the white-space changes and the roxygen2-induced changes made this noisier but we coped :)

Thank you both. Will ship a new version to CRAN.

@eddelbuettel
Copy link
Owner

Thanks again -- new version sailed to CRAN without an issue (as we're all 'OK' on each platform and have no reverse depends). So all good now :)

Congrats to Christoph for a fine first PR (just hold back the whitespace changes next time ;-) and thanks again also to Jan for looking over my shoulder.

Updated code for version 0.1.5 in master now too.

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.

None yet

3 participants