ropensci / bibtex

bibtex parser for R
https://docs.ropensci.org/bibtex/
36 stars 12 forks source link

Remove C code #47

Closed dieghernan closed 2 years ago

dieghernan commented 2 years ago

Hi:

Following #46 and related to #45 , this PR removes the C code of the package, substituting it by pure R (base) code. I am aware this is a critical change to the package, so let me share some points of attention on this PR:


So `Rnews2001-3` should be parsed as  `rnews # "Rnews_2001-3.pdf` (http://CRAN.R-project.org/doc/Rnews/Rnews_2001-3.pdf)

- I run `revdepcheck::revdep_check()` succesfully. This is specially important for maintainers of the affected packages (@renozao, @sckott, @njahn82, @bwiernik, @mwmclean )

Comments welcome

This would also close #42 , fix #16
dieghernan commented 2 years ago

I added some comments on the review tab to make easier to understand some things @coatless

dieghernan commented 2 years ago

Thanks for this useful review. I would set this as a Draft until I tackle all your comments

dieghernan commented 2 years ago

Hi @coatless , I think I tackled all your comments, ready for review

dieghernan commented 2 years ago

Hi @coatless , just a quick reminder on this

maelle commented 2 years ago

:wave: @coatless @dieghernan, what's the status on this? Thanks both for your work on {bibtex} :pray:

dieghernan commented 2 years ago

Hi @maelle

I re-runned checks, examples and snapshots after 8 months and I see no problems, so this is ready on my side. This PR would close #16 and close #42

maelle commented 2 years ago

Thanks @dieghernan! @coatless would you be able to review this PR?

coatless commented 2 years ago

@dieghernan thanks for addressing the remaining issues. Sorry this sat for so long!