Skip to content

Sfp-115-nl veldprotocol abiotiek oppervlaktewater WTW Multi 3430 veldmeter #113

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

Merged
merged 26 commits into from
Jun 3, 2024

Conversation

Florian9041
Copy link
Contributor

@Florian9041 Florian9041 commented May 30, 2024

Description

Review van SVP-115 voor publicatie

Related Issue

Bij afhankelijkheden vermoed ik dat alleen verwijzing naar SVP-114 (dat nu gepubliceerd is) mag blijven staan?

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.

Comment on lines 3 to 12
Procedures waarnaar in deze procedure verwezen wordt:

- SIP-055B: Veldmeter voor pH, EC en O2, WTW Multi 3430 (in opmaak)

- SVP-015: Bioveiligheidsmaatregelen (in opmaak)

- SVP-112: Veiligheid in en rond water (in opmaak)

- SVP-114: Bemonstering waterkolom oppervlaktewater

Copy link
Collaborator

@hansvancalster hansvancalster May 31, 2024

Choose a reason for hiding this comment

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

Al deze tekst moet je inderdaad verwijderen. Afhankelijkheden worden automatisch in de tabel (zie code chunk met label dependencies) opgesomd als je deze toegevoegd hebt met protocolhelper::add_dependencies.
Enkel sfp-114-nl is gepubliceerd. Je kan de volgende code gebruiken om deze dependency toe te voegen aan je protocol:

protocolhelper::add_dependencies(
    code_mainprotocol = "spp-115-nl",
    protocol_code = "sfp-114-nl",
    version_number = "2024.03",
    params = NA,
    appendix = FALSE
)

Naar de niet gepubliceerde protocols kan je in de tekst zelf verwijzen zoals we dat in protocol sfp-114-nl hebben gedaan met syntax voor een weblink. Dus bv [SIP-055B](hier de juist url).

Choose a reason for hiding this comment

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

Moeten de procedures die verwijzen naar deze procedure ook opgesomd worden? In het originele document staan SPP-116: Abiotische staalname stilstaande wateren en SPP-117: Abiotische staalname stromende wateren vermeld. Maar ik kan begrijpen dat dit overbodig is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik krijg de volgende foutmelding als ik bovenstaande code in de console probeer te runnen:
"Error in get_path_to_protocol(code_mainprotocol) :
Do you want to get the path?
If so, check if the protocol code is correct.
If you want to set a path for a new protocol,
you need to provide a short title"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Zo te zien heb ik in bovenstaande code een foutje gemaakt: het moet sfp-115-nl zijn in plaats van spp-115-nl

Copy link

@anleyssen anleyssen left a comment

Choose a reason for hiding this comment

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

Ziet er goed uit! Maar ik heb een paar kleine suggesties.

Comment on lines 3 to 12
Procedures waarnaar in deze procedure verwezen wordt:

- SIP-055B: Veldmeter voor pH, EC en O2, WTW Multi 3430 (in opmaak)

- SVP-015: Bioveiligheidsmaatregelen (in opmaak)

- SVP-112: Veiligheid in en rond water (in opmaak)

- SVP-114: Bemonstering waterkolom oppervlaktewater

Choose a reason for hiding this comment

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

Moeten de procedures die verwijzen naar deze procedure ook opgesomd worden? In het originele document staan SPP-116: Abiotische staalname stilstaande wateren en SPP-117: Abiotische staalname stromende wateren vermeld. Maar ik kan begrijpen dat dit overbodig is.

Copy link
Contributor Author

@Florian9041 Florian9041 left a comment

Choose a reason for hiding this comment

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

Wijzigingen doorgevoerd, alleen "add_dependencies" voor verwijzing naar SVP-114 lijkt niet te werken?

Comment on lines 3 to 12
Procedures waarnaar in deze procedure verwezen wordt:

- SIP-055B: Veldmeter voor pH, EC en O2, WTW Multi 3430 (in opmaak)

- SVP-015: Bioveiligheidsmaatregelen (in opmaak)

- SVP-112: Veiligheid in en rond water (in opmaak)

- SVP-114: Bemonstering waterkolom oppervlaktewater

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ik krijg de volgende foutmelding als ik bovenstaande code in de console probeer te runnen:
"Error in get_path_to_protocol(code_mainprotocol) :
Do you want to get the path?
If so, check if the protocol code is correct.
If you want to set a path for a new protocol,
you need to provide a short title"

@hansvancalster
Copy link
Collaborator

@Florian9041 kan je ook nog eens 'git pull origin main' uitvoeren in de terminal (zorg er wel voor dat je git venster leeg is vooraleer je dit doet, dus geen staged, modified of added bestanden), want deze branch zit achter ten opzichte van de main branch.

@Florian9041
Copy link
Contributor Author

Florian9041 commented Jun 3, 2024 via email

@hansvancalster hansvancalster merged commit 207760d into main Jun 3, 2024
3 checks passed
@hansvancalster hansvancalster deleted the sfp-115-nl branch June 3, 2024 16:08
@hansvancalster hansvancalster restored the sfp-115-nl branch June 4, 2024 08:35
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