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.

Sign up for GitHub

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

Jump to bottom

standardize/style code formatting #117

Merged
merged 4 commits into from
May 25, 2020
Merged

standardize/style code formatting #117

merged 4 commits into from
May 25, 2020

Conversation

eoduniyi
Copy link
Collaborator

Used styler (i.e., style_dir("R/",...)) package to adhere to tidyverse default consistent code style.

@eoduniyi
standardize/style code formatting
a4cb3ce 
Used styler (i.e., style_dir("R/",...)) package to adhere to tidyverse default consistent code style.
@bryanhanson
Copy link
Collaborator

bryanhanson commented May 12, 2020 via email

@eoduniyi
Copy link
Collaborator Author

Yes, and I remember a while back (sometime around when I first submitted the pull request #107) I tried to build after styling but ran into the same issue #111...just tried building again a few minutes ago and the same thing happened.

@eoduniyi
Copy link
Collaborator Author

Erick's pull request on code styling

@bryanhanson
Copy link
Collaborator

I will review once #111 is resolved and I can B & C locally.

ximeg
ximeg approved these changes May 12, 2020
View reviewed changes
Copy link
Collaborator

@ximeg ximeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I did not check the code thoroughly, but on the first glance it looks fine. Since it was automatically generated, it's probably fine.

I checked out this branch locally and merged with the pull request #118 to have the correct build system. I could build everything with make bootstrap and invoked devtools::test('hyperSpec/') to run all unit tests.

══ Results ════════════════════════════════════════════════════════
Duration: 30.7 s

OK:       517
Failed:   0
Warnings: 0
Skipped:  8

Great job, everything works! 🥇

cbeleites
cbeleites approved these changes May 13, 2020
View reviewed changes
Copy link
Owner

@cbeleites cbeleites left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I found a few files where spacing to align things was not recognized by styler::style(), but I think it's easier to take care of that after this monster-commit is merged.

The files/functions are:

  • pooled.cov()
  • makeraster()
  • the generic plot()
  • read.spe.R

@eoduniyi
Copy link
Collaborator Author

Looks good to me. I found a few files where spacing to align things was not recognized by styler::style(), but I think it's easier to take care of that after this monster-commit is merged.

The files/functions are:

  • pooled.cov()
  • makeraster()
  • the generic plot()
  • read.spe.R

WOW, nice catch!

@GegznaV
Copy link
Collaborator

GegznaV commented May 20, 2020

A few remarks:

Since it was automatically generated, it's probably fine. (@ximeg)

The author of styler package Yihui Xie thinks, that after automatic corrections the code must be thoroughly checked (by unit test or manually). But as unit test did not fail, it should be OK.

I think it's easier to take care of that after this monster-commit is merged. (@cbeleites)

I agree.

@eoduniyi eoduniyi merged commit 9b3fd94 into develop May 25, 2020
@ximeg ximeg deleted the issue/96-Consistent-code-style branch May 25, 2020 15:13
@GegznaV GegznaV added the Topic: style ✏️ Related to code style (documentation, R scripts, etc.) label Jun 22, 2020
@GegznaV GegznaV mentioned this pull request Jun 22, 2020
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbeleites cbeleites cbeleites approved these changes

@ximeg ximeg ximeg approved these changes

@bryanhanson bryanhanson Awaiting requested review from bryanhanson

Assignees
No one assigned
Labels
Topic: style ✏️ Related to code style (documentation, R scripts, etc.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eoduniyi @bryanhanson @GegznaV @cbeleites @ximeg