extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
186 stars 27 forks source link

Update ui functions #94

Closed malcolmbarrett closed 3 years ago

malcolmbarrett commented 3 years ago

This is the first of two PRs that I plan to submit to address #62. Doesn't quite close that issue.

This PR:

Shoutout to @Ilia-Kosenkov who laid the groundwork for this PR!

clauswilke commented 3 years ago

Could you add a file with developer guidelines summarizing how these functions should be used? Along the lines of this file in extendr: https://github.com/extendr/extendr/blob/master/MAINTAINERS_GUIDE.md

malcolmbarrett commented 3 years ago

@clauswilke what do you think about putting it in a CONTRIBUTING.md generated by usethis::use_tidy_contributing()?

clauswilke commented 3 years ago

We have contributing guidelines over at extendr and they could be adapted: https://github.com/extendr/extendr/blob/master/CONTRIBUTING.md

However, I think that's not the place for documentation that would cover internal coding conventions. I'm not sure where that would go, though. Most projects don't have such documentation and it's often a major problem, as nobody knows how exactly code should be written.

malcolmbarrett commented 3 years ago

Yeah, agreed. usethis has a helpful document along those lines here: https://github.com/r-lib/usethis/blob/master/principles.md and in fact has a section on communicating with the user

clauswilke commented 3 years ago

Yes, exactly, something like this.

malcolmbarrett commented 3 years ago

Ok, I added a principles.md document that includes details on UI and errors. I'll file an issue to include other items outside the scope of this PR

clauswilke commented 3 years ago

One more thing that I can't check easily from my phone right now: did you add the principles file to .Rbuildignore?

malcolmbarrett commented 3 years ago

yes, already done

Ilia-Kosenkov commented 3 years ago

Is there anything blocking this PR?

clauswilke commented 3 years ago

Not from my end.

Ilia-Kosenkov commented 3 years ago

@malcolmbarrett , you should probably merge this.

malcolmbarrett commented 3 years ago

I can't. I don't have access 😎

Ilia-Kosenkov commented 3 years ago

@malcolmbarrett, oh shoot, I had no idea, I am sorry. If you think it is ready, I can merge it.

malcolmbarrett commented 3 years ago

No worries! Yes, I think it is ready

malcolmbarrett commented 3 years ago

Thanks!