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

Uploading plant functional type species classification base R script #23

Merged
merged 6 commits into from
Apr 1, 2025

Conversation

arne-exe
Copy link
Collaborator

@arne-exe arne-exe commented Mar 19, 2025

This pull request is linked to #22 and uploads the plant functional type species classification base R script to the analysis folder.

Copy link
Collaborator

@davidorme davidorme left a comment

Choose a reason for hiding this comment

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

This is looking soild @arne-exe - some minor issues with absolute file paths.

Copy link
Collaborator

@davidorme davidorme left a comment

Choose a reason for hiding this comment

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

Thanks - really useful working through of the process and has brought up some important general points about working with the repo. The branch naming and the # nolint are things we need to let everyone know about.

I'd change the filepath back as shown below.

@arne-exe arne-exe requested review from davidorme and annarallings and removed request for davidorme March 26, 2025 08:08
Copy link
Collaborator

@annarallings annarallings left a comment

Choose a reason for hiding this comment

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

LGTM

@arne-exe
Copy link
Collaborator Author

arne-exe commented Mar 26, 2025

Hi @davidorme, could you please approve the pull request. The requested changes were made but I think your approval of these changes are the final thing preventing the merge (even when Anna has approved the pull request). Could you also please explain if this is indeed the correct way (i.e., if a reviewer requests changes they HAVE to explicitly approve it again before the pull request can be merged)?

@arne-exe arne-exe requested a review from davidorme March 28, 2025 07:58
@annarallings
Copy link
Collaborator

Have these changes been made to your satisfaction @davidorme?

Copy link
Collaborator

@davidorme davidorme left a comment

Choose a reason for hiding this comment

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

LGTM

@arne-exe arne-exe merged commit 6cb5ab8 into main Apr 1, 2025
2 checks passed
@arne-exe
Copy link
Collaborator Author

arne-exe commented Apr 1, 2025

Issue #22 can now be closed.

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