yihui / xfun

Miscellaneous R functions
https://yihui.org/xfun/
Other
141 stars 28 forks source link

2 unrelated improvements: 1. base_pkgs() can "hang". 2. `isFALSE()` is faster and better in `base`. #66

Closed mmaechler closed 2 years ago

mmaechler commented 2 years ago
  1. I found that knitr::write_bib() would hang the R process for me. Found then that it was when xfun::base_pkgs() was called. The reason is I have 20'000 packages installed in my .libPaths(). Instead, the base packages are always in .Library hence 197e3575f741f9401d0a48901971b93b0b357d5c.

  2. I saw your isFALSE(). Presumably it is older than the one in {base} which is also a few years old. When we introduced it (I was involved with it), we found we do not want identical, because a FALSE (or TRUE) can also end up with names or other such attributes. The change cfbf040 keeps xfun's isFALSE() local, i.e, does just not export it. Personally I think you should even not define it anymore, but rather use the base one for consistency.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

yihui commented 2 years ago

For isFALSE(), I'll deal with it in a separate PR, since this is not a trivial decision to make like the one on using .Library (it's better to send separate patches in separate PRs so that the easier ones can be quickly merged first).

mmaechler commented 2 years ago

... (it's better to send separate patches in separate PRs so that the easier ones can be quickly merged first).

Yes, I'm sorry for the extra work you had.
{Somehow the github web interface allowed me to do one PR from the two changes in my clone (mmaechler/xfun which was "ahead of" yours by 2 commits) easily, and I tried a while to do separate PRs instead, but did not see how ..

yihui commented 2 years ago

No worries at all! I really appreciate your contribution!

If you make the changes in separate branches, you can then submit a separate PR for each branch. For the Github web interface, you can click on the edit button on the toolbar of a file (open a file on Github and find the pencil button on the right) to suggest changes:

the edit button

Then the web interface will guide you to create a new branch for the changes that you made.

I guess that you did was first forking the repo, then making the changes in the main branch, and finally submitting the PR. All changes were in the same branch, so you could only submit one PR.