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

Sfp 201 nl #41

Merged
merged 76 commits into from
Jan 8, 2024
Merged

Sfp 201 nl #41

merged 76 commits into from
Jan 8, 2024

Conversation

Nathalie-Cools
Copy link
Contributor

@Nathalie-Cools Nathalie-Cools commented May 12, 2021

Description

Omzetting Word versie van het protocol naar een eerste online versie

Related Issue

Task list

Steps by contributor:

  • Add description to this pull request (under "## Description")
  • Open the dropdown (triangle) near 'create pull request' and choose 'Create draft pull request'
  • Check for potential problems by running protocolhelper::check_frontmatter() and address them
  • Check for potential problems by running protocolhelper::check_structure() and address them
  • Add further commits if needed and push them to GitHub
  • Update the protocol-specific NEWS.Rmd
  • Mark the pull request as 'ready for review'

Review steps for the author(s):

  • Add reviewers, at least one subject-matter specialist and one administrator
  • Wait for review comments and address them
  • Iterate until reviewer approvals (merging the pull request will be done by an administrator)
  • Verify that the checks done by continuous integration succeeded. These will check if protocolhelper::check_frontmatter() and protocolhelper::check_structure() succeeded without errors.

To be done by an administrator after review: see guidelines for admins.

@Nathalie-Cools Nathalie-Cools marked this pull request as draft May 12, 2021 13:10
@hansvancalster
Copy link
Collaborator

@Nathalie-Cools ik ben begonnen met parametrisatie van het protocol. Zie in de YAML sectie in index.Rmd bij params om te weten welke parameters ik heb gedefinieerd voor dit protocol. Deze parameters kunnen nu in de rest van het protocol gebruikt worden. De waarden kunnen opgeroepen worden door params$naamvandeparameter in een R chunk, ofwel `r params$naamvandeparameter` in een inline R expressie (dus om de waarde te gebruiken in een zin).

@hansvancalster
Copy link
Collaborator

@Nathalie-Cools ik heb ondertussen alle kruisverwijzingen in orde gebracht. De eerste bijlage heb ik aangepast. De verwijzingen naar specifieke projecten (habnorm, ...) zijn weggehaald (omdat dit het generiek protocol is, voor HabNorm kan dan een projectspecifieke variant gemaakt worden). Hierdoor heb ik ook de afmetingen van het proefvlak volledige weggehaald. Misschien is het beter dat we die terug toevoegen als een INBO default (bv 3 x 3 m in open vegetatie en 16 m x 16 m in bosvegetatie als we dezelfde standaard volgen als bij vegetatie-opnamen)?

Kan je de html versie nakijken (protocolhelper::render_protocol("sfp-201-nl") ofwel onderaan bij artifacts downloaden) en ofwel zelf verder aanpassingen doen ofwel hier aangeven wat er nog moet veranderd worden?

Voor bijlage B heb ik ook nog de pdf nodig van het opnameformulier. Ik zal dan zorgen dat die via een link kan gedownload worden.

@Nathalie-Cools
Copy link
Contributor Author

Kan ik hier zo bijlage B uploaden met het opnameformulier?
SVP-201_bijlage 2 Opnameformulier_Oppervlaktemonsters.pdf

@hansvancalster
Copy link
Collaborator

Kan ik hier zo bijlage B uploaden met het opnameformulier?
SVP-201_bijlage 2 Opnameformulier_Oppervlaktemonsters.pdf

Ja, perfect!

@hansvancalster
Copy link
Collaborator

De link in bijlage B naar het opnameformulier is in orde (zie 88be092)

@Nathalie-Cools
Copy link
Contributor Author

OK. Ik ben al blij dat ik de bijlage kan zien!

Nu heb ik ook één zinnetje aangepast in het rmd bestand 'stappenplan.rmd'.

Daar was een zinnetje met betrekking tot de bemonsterinsgdiepte teveel veralgmeend nl. oorsponkelijk stond er
"Dit protocol kan op analoge wijze toegepast worden op diepte van 0 -20 cm of 0 – 30 cm."
Jij had ervan gemaakt: "Dit protocol kan op analoge wijze toegepast worden op andere diepten".
Daar staat beter "Dit protocol kan op analoge wijze toegepast worden op andere diepten startend van het bodemoppervlak".
Ik heb dat dus zo aangepast en terug gepusht. Is dat nu in orde?

@hansvancalster
Copy link
Collaborator

Ik heb dat dus zo aangepast en terug gepusht. Is dat nu in orde?

Als het gelukt was, dan zou je op deze pagina je commit zien verschijnen. Het is dus niet gelukt. Dit zijn normaal de stappen die je zou moeten doen:

  • rstudio project opstarten
  • check dat je in de juiste branch zit (git pane)
  • pull knop drukken om de laatste wijzigingen binnen te halen
  • aanpassing doen in Rmd en opslaan
  • in git pane staat nu het bestand dat je gewijzigd hebt. Vink dit aan en druk op de commit knop
  • vul een commit message in en druk op commit
  • daarna druk je op de push knop om je lokale wijziging naar hier te krijgen

Copy link
Contributor

@cecileherr cecileherr left a comment

Choose a reason for hiding this comment

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

@Nathalie-Cools

Ooit, (te) lang geleden, had je mij gevraagd om dit protocol nog eens na te lezen. Sorry voor de late reactie!

Het lukte me niet direct om de code te laten lopen en ik zag ook geen PDF/html in de discussie, ik heb dus rechtstreeks in de Rmd's gekeken en hier en daar een kleine opmerking/suggestie toegevoegd.

@hansvancalster
Copy link
Collaborator

@Nathalie-Cools kan je de twee openstaande opmerkingen van Cécile bekijken. De andere heb ik al aangepakt + je branch terug in orde gezet (stond achter tov de main branch).

@hansvancalster hansvancalster self-requested a review January 8, 2024 12:32
@hansvancalster hansvancalster merged commit 2a823cb into main Jan 8, 2024
3 of 4 checks passed
@hansvancalster hansvancalster deleted the sfp-201-nl branch January 8, 2024 12:50
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.

3 participants