Skip to content

Dev - #19

Open
Tefera19 wants to merge 26 commits into
masterfrom
dev
Open

Dev#19
Tefera19 wants to merge 26 commits into
masterfrom
dev

Conversation

@Tefera19

Copy link
Copy Markdown
Collaborator

This PR contains additional vigneets and {stamp} implementation.

@randrescastaneda randrescastaneda left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @Tefera19 ,

Thank you for the PR. Unfortunatley, there are something that I still don't understad. Could you please address those comments? Also, could you please make sure that the stamp suite is fully tested in the unit tests? You can create mock functions or data if needed.

Comment thread README.html Outdated
Comment thread R/dlw_get_data.R
@Tefera19

Copy link
Copy Markdown
Collaborator Author

Hi @randrescastaneda ,

I just wanted to let you know that I've deleted README.html file. I also created unit test for dlw_get_gmd(). The test unit shows how to setup {stamp} to save dlw data.

@bbrunckh bbrunckh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi, coming on to help maintain and took a quick look at this one.

It is not obvious to me exactly what problem {stamp} is fixing here other than replacing {pins}. Lighter weight? I know there are advantages for tracking files in a more complex data pipeline, but not sure how applies here for datalibweb only or where {pins} was deficient.

@Tefera19 are you able to add a brief comment to explain?

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