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

Sfp 105 nl #53

Merged
merged 51 commits into from
Dec 12, 2023
Merged

Sfp 105 nl #53

merged 51 commits into from
Dec 12, 2023

Conversation

ToonVanDaele
Copy link
Collaborator

@ToonVanDaele ToonVanDaele commented May 10, 2022 •
edited
Loading

Description

Add new protocol sfp-105-nl. Imported from docx

Related Issue

Task list

Steps by contributor:

  • Add description to this pull request (under "## Description")
  • Submit the pull request by clicking 'create pull request'
  • Mark the pull request as draft
  • Check whether the version number in the index.Rmd yaml section is of format yyyy.nn.dev
  • In case of a protocol created from a pre-existing docx protocol, check if all sections comply with current template for a new protocol
  • 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)

To be done by an administrator after review, but before merging the pull request:

  • Update the .zenodo.json file (authorship will extend as protocols are added)
  • Bump the version number in the index.Rmd yaml section from yyyy.nn.dev to YYYY.NN.
  • Update the repo NEWS.md file

To be done by an administrator after merging this pull request:

  • Add general tag to the merge commit (protocols-YYYY.NN) (see release model)
  • Add specific tag to the merge commit (<protocol-code>-YYYY.NN) (see release model)
  • Check whether continuous integration proceeded without problems
  • Create a new release based on a general tag
  • Check success of publication steps at protocols.inbo.be and at Zenodo (records both source and rendered)

@hansvancalster
Copy link
Collaborator

Ik zag hier twee bestanden temp.Rmd en temp2.Rmd, kan je deze verwijderen (eventueel nadat je relevante inhoud naar één van de andere bestanden hebt gekopieerd?).

hansvancalster
hansvancalster reviewed May 18, 2022
View reviewed changes
src/thematic/1_water/sfp-105-nl_bemonsteren-peilbuis/03_beperkingen.Rmd Outdated
```
De bemonstering moet worden uitgevoerd in een geschikte peilbuis in PVC of HDPE, afhankelijk van de aanwezige stoffen in het grondwater en de te analyseren chemische variabelen

De procedure is niet geschikt voor het bemonsteren van grondwater met een zeer hoge saliniteit (> xxxx µS/cm)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kan hier een waarde op worden geplakt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ik heb een comment line toegevoegd. Dat moeten we met labo en milklim bespreken. Mogelijk zijn er nog andere beperken.

hansvancalster
hansvancalster reviewed May 18, 2022
View reviewed changes
src/thematic/1_water/sfp-105-nl_bemonsteren-peilbuis/07_stappenplan.Rmd Outdated
Comment on lines 46 to 50
Bij het nemen van een monster met een peristaltische pomp, wordt een teflondarm tot op een diepte van 10 cm onder het waterpeil van de peilbuis gebracht.
De teflondarm wordt verbonden met de peristaltische pomp en het 'vers' grondwater wordt met een matige snelheid opgepompt zodat geen gasbellen in het opgepompte water terechtkomen.
Er moet absoluut vermeden worden dat de peilbuis leeggepompt wordt tijdens de staalname.
Het opgepompte water wordt opgevangen in een recipiënt (monsternamepot) totdat een volume van minstens 500 ml is bereikt.
Bij voldoende wateraanvoer in de peilbuis wordt de eerste 100 ml niet opgevangen.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is parametrisatie van deze cijfers zinvol / nodig? Je kan aangeven dat deze cijfers de INBO defaultwaarden zijn, maar dat daar in projectspecifieke context van kan afgeweken worden als dat nodig is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ik denk niet dat hier ooit van afgeweken wordt. Ik neem het mee in het lijstje te bediscusiëren punten

hansvancalster
hansvancalster reviewed May 18, 2022
View reviewed changes
Copy link
Collaborator

@hansvancalster hansvancalster left a comment

Choose a reason for hiding this comment

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

This PR removes this file from_docx/SVP_401_VegetatieOpnamePV_Terrestrisch_v1.1.docx from the main branch. Can you revert this? Is your branch aligned with the main branch? This is also the case for your other PRs.

@hansvancalster
Copy link
Collaborator

@ToonVanDaele kan je de openstaande comments nog afhandelen? Voor de rest lijkt het mij klaar. Ik heb zelf nog wat kleine verbeteringen gedaan (zie 7108987 en de drie voorgaande commits)

@ToonVanDaele
Copy link
Collaborator Author

ToonVanDaele commented Nov 20, 2023 •
edited
Loading

This PR removes this file from_docx/SVP_401_VegetatieOpnamePV_Terrestrisch_v1.1.docx from the main branch. Can you revert this? Is your branch aligned with the main branch? This is also the case for your other PRs.

Dag Hans, dit bestand wordt in de eerste commit van de branch verwijderd. Hoe kan ik dit ongedaan maken?

@hansvancalster
Copy link
Collaborator

This PR removes this file from_docx/SVP_401_VegetatieOpnamePV_Terrestrisch_v1.1.docx from the main branch. Can you revert this? Is your branch aligned with the main branch? This is also the case for your other PRs.

Dag Hans, dit bestand wordt in de eerste commit van de branch verwijderd. Hoe kan ik dit ongedaan maken?

Dat lijkt mij niet meer nodig? Bij changed files staat dit bestand niet meer erbij, dus het probleem waarvan ik hierboven melding maakt is blijkbaar al opgelost.

mathiaswackenier
mathiaswackenier approved these changes Nov 21, 2023
View reviewed changes
Copy link

@mathiaswackenier mathiaswackenier left a comment

Choose a reason for hiding this comment

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

De recipiënten zijn het afgelopen jaar gewijzigd, maar de wijziging is nog niet doorgevoerd in SOP-006. Ik zal de specifieke types opvragen bij het labo en een issue voor aanmaken. We kunnen dit dan verwerken in een revisie.

@ToonVanDaele ToonVanDaele marked this pull request as ready for review December 4, 2023 13:03
@hansvancalster hansvancalster merged commit a4ee3c4 into main Dec 12, 2023
19 of 20 checks passed
@hansvancalster hansvancalster deleted the sfp-105-nl branch December 12, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hansvancalster hansvancalster hansvancalster approved these changes

@mathiaswackenier mathiaswackenier mathiaswackenier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ToonVanDaele @hansvancalster @mathiaswackenier