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

Veldprotocol Sfp-114-nl naar main #107

Merged
merged 22 commits into from
May 27, 2024
Merged

Veldprotocol Sfp-114-nl naar main #107

merged 22 commits into from
May 27, 2024

Conversation

Florian9041
Copy link
Contributor

@Florian9041 Florian9041 commented May 17, 2024 •
edited by hansvancalster
Loading

Description

sfp-114-nl is klaar om naar main overgezet te worden.
Moet nog gereviewed worden voor finaal online zetten vermoed ik?

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.

@Florian9041
in sfp-114 branch:
2eec343 
- template-hoofdstukken aangemaakt
- template-hoofdstukken ingevuld
- afbeeldingen toegevoegd
@Florian9041
Bumped 2024.11 to 2024.03 in index.Rmd and NEWS.md
10b8fcf
anleyssen
anleyssen reviewed May 22, 2024
View reviewed changes
source/sfp/1_water/sfp_114_nl_waterstaalname_plas/NEWS.md Outdated
Copy link

Choose a reason for hiding this comment

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

dit is toch de eerste versie? er zijn toch geen vorige versies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

protocolhelper vraagt automatisch om het versienummer te updaten bij check_all(), dus dit heb ik gedaan bij update_version_number()

Copy link

Choose a reason for hiding this comment

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

Misschien door @hansvancalster ook eens te bekijken? Nu staat er in de tabel met metadata versienummer 2024.03 vermeld, maar dit nummer staat ook vermeld als 'vorige versienummer'. Klopt dit? Ik zou bij vorige versies onder 1.1 wat tekst verwachten, zoals 'geen vorige versies' of 'dit is de eerste versie'? in bijlage de pdf die florian compileerde
sfp_114_nl_waterstaalname_plas.pdf

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ik heb de NEWS aangepast

anleyssen
anleyssen requested changes May 22, 2024
View reviewed changes
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.

Dag Florian, Ik denk dat er nog een beetje werk is bij de bijschriften en verwijzingen van figuren en tabellen, maar voor de rest ziet het er ok uit!

@Florian9041 Florian9041 requested a review from anleyssen May 24, 2024 07:45
anleyssen
anleyssen requested changes May 27, 2024
View reviewed changes
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.

Prima, de link met de figuren en andere aanpassingen zijn goed gelukt! Echter nog één opmerking: helemaal op het eind van de gecompileerde pdf staat bij Referenties 'niet van toepassing'. Dit mag verwijderd worden; dan is voor mij alles ok.
sfp_114_nl_waterstaalname_plas.pdf

@Florian9041
Copy link
Contributor Author

Florian9041 commented May 27, 2024 via email

@hansvancalster
kruisverwijzing tabel
a43acc0
@hansvancalster
merge sfp-114-nl
5f6fd15 
Merge branch 'sfp-114-nl' of https://github.com/inbo/protocolsource into sfp-114-nl-review-hvc

# Conflicts:
#	source/sfp/1_water/sfp_114_nl_waterstaalname_plas/07_werkwijze.Rmd
@hansvancalster
Merge pull request #108 from inbo/sfp-114-nl-review-hvc
069accf 
Sfp 114 nl review hvc
@Florian9041 Florian9041 requested a review from anleyssen May 27, 2024 08:22
anleyssen
anleyssen approved these changes May 27, 2024
View reviewed changes
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 ok uit!

@hansvancalster hansvancalster merged commit 810f606 into main May 27, 2024
3 of 4 checks passed
@hansvancalster hansvancalster deleted the sfp-114-nl branch May 27, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anleyssen anleyssen anleyssen approved these changes

@hansvancalster hansvancalster hansvancalster 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
@Florian9041 @hansvancalster @anleyssen