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

Closes #11 : vs_metabolic test data draft created #17

Closed
wants to merge 12 commits into from

Conversation

Siddhesh2097
Copy link
Contributor

@Siddhesh2097 Siddhesh2097 commented Jul 29, 2024

Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral family codebase remains robust and consistent.

Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the main branch until you have checked off each task.

  • Place Closes #<insert_issue_number> into the beginning of your Pull Request Title (Use Edit button in top-right if you need to update)
  • Code is formatted according to the tidyverse style guide. Run styler::style_file() to style R and Rmd files
  • Updated relevant unit tests or have written new unit tests, which should consider realistic data scenarios and edge cases, e.g. empty datasets, errors, boundary cases etc. - See Unit Test Guide
  • If you removed/replaced any function and/or function parameters, did you fully follow the deprecation guidance?
  • Update to all relevant roxygen headers and examples, including keywords and families. Refer to the categorization of functions to tag appropriate keyword/family.
  • Run devtools::document() so all .Rd files in the man folder and the NAMESPACE file in the project root are updated appropriately
  • Address any updates needed for vignettes and/or templates
  • Update NEWS.md under the header # admiral<ext> (development version) if the changes pertain to a user-facing function (i.e. it has an @export tag) or documentation aimed at users (rather than developers)
  • Build site pkgdown::build_site() and check that all affected examples are displayed correctly and that all new functions occur on the Reference page.
  • Address or fix all lintr warnings and errors - lintr::lint_package()
  • Run R CMD check locally and address all errors and warnings - devtools::check()
  • Link the issue in the Development Section on the right hand side.
  • Address all merge conflicts and resolve appropriately
  • Pat yourself on the back for a job well done! Much love to your accomplishment!

Copy link

github-actions bot commented Jul 29, 2024

Code Coverage

Package Line Rate Health
admiralmetabolic 100%
Summary 100% (3 / 3)

@Siddhesh2097 Siddhesh2097 linked an issue Jul 29, 2024 that may be closed by this pull request
3 tasks
@Siddhesh2097 Siddhesh2097 marked this pull request as draft July 29, 2024 05:17
@Siddhesh2097 Siddhesh2097 changed the title #11 : vs_metabolic test data draft created Closes #11 : vs_metabolic test data draft created Jul 29, 2024
@manciniedoardo
Copy link
Collaborator

@Siddhesh2097 i guess we hold off on this until we have @AndersAskeland merge in something for #12 ? As after that you can then use dm_metabolic directly.

@Siddhesh2097
Copy link
Contributor Author

@Siddhesh2097 i guess we hold off on this until we have @AndersAskeland merge in something for #12 ? As after that you can then use dm_metabolic directly.

Yes @manciniedoardo , we'll merge once dm_metabolic is ready

@manciniedoardo
Copy link
Collaborator

@Siddhesh2097 - #12 is now closed and merged, so please:

  1. merge main into your feature branch
  2. resolve any conflicts
  3. update your script to use dm_metabolic
  4. check the failing code style and lintr checks to see if you can fix them (happy to support if not)

Thanks!

@Siddhesh2097
Copy link
Contributor Author

@manciniedoardo
Can you help me in resolving the checks ? I tried but I'm missing something.

@manciniedoardo
Copy link
Collaborator

Hi @Siddhesh2097, something funny seems to be happening here as your feature branch is way behind with respect to what is in main. I can see this because your PR has a lot more changes than I would expect. Can you try running git checkout 11_create_vs_metabolic && git rebase main and then pushing to see what happens?

@Siddhesh2097
Copy link
Contributor Author

@manciniedoardo ,

image

@manciniedoardo
Copy link
Collaborator

@manciniedoardo ,

image

Ok, try remove them, no big deal if something goes wrong as we are working in a feature branch

@Siddhesh2097
Copy link
Contributor Author

@manciniedoardo ,
image

Ok, try remove them, no big deal if something goes wrong as we are working in a feature branch

Okay, I deleted those two files and ran a git pull origin main . These are the files in my git currently after pulling. Earlier , I had restored the files that I didn't work on and didn't want to commit those. Doing that, I realized there were few updates from main that were required and I manually re-updated them. This is the reason, we are seeing that feature branch is way behind w.r.t main branch.

image

@manciniedoardo
Copy link
Collaborator

@manciniedoardo ,
image

Ok, try remove them, no big deal if something goes wrong as we are working in a feature branch

Okay, I deleted those two files and ran a git pull origin main . These are the files in my git currently after pulling. Earlier , I had restored the files that I didn't work on and didn't want to commit those. Doing that, I realized there were few updates from main that were required and I manually re-updated them. This is the reason, we are seeing that feature branch is way behind w.r.t main branch.

image

ok, looks good, could you try resolve the conflict in data.R appropriately by updating the file to what you think it should look like, and then push?

@manciniedoardo
Copy link
Collaborator

Hmm. something is still wrong as the changes asre still present. Not sure what happened here. I think the easiest and fastest solution would be if you create a new branch off of main and paste your code in there, then make a PR for that branch and close this one.

@AndersAskeland AndersAskeland deleted the 11_create_vs_metabolic branch September 2, 2024 08:44
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.

Test data: vs_metabolic
2 participants