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

Merge @thomasyu888's updates to use Golem for Shiny app structure #2

Merged
merged 16 commits into from
May 31, 2021

Conversation

jaeddy
Copy link
Collaborator

@jaeddy jaeddy commented May 31, 2021

This PR merges Tom's various changes to repo structure to conform with a standard Golem-based Shiny app, including some additional functionality for handling Synapse login.

@jaeddy jaeddy merged commit d63ac79 into develop May 31, 2021
@jaeddy jaeddy deleted the use-golem branch May 31, 2021 06:47
@thomasyu888
Copy link
Member

Testing this branch: this might be somewhat cumbersome to set up initially, but i think would be easier to adapt/extend moving forward (and you could probably get it working a lot more quickly than me anyway):

  1. There are 3 files and instructions for how to validate/upload manifests here; along with another for publications attached
  2. if you were to subset those (maybe not the additional_standard_terms sheet...), then do a Synapse tableQuery to grab the matching rows from the Publications, Datasets, and Tools and pull the CSVs/dfs
  3. ideally, there should be a way to run a test for each of the manifest files without needing to use the app itself or upload the final results to Synapse (e.g., my comparing the ready-to-upload data to the CSV/df rows)
  4. if everything matches, then you at least know that the tool is still working the same after your changes
  5. if they don't match, then we'd need to take a closer look... in case you actually fixed a bug somewhere

@jaeddy
Copy link
Collaborator Author

jaeddy commented Jun 1, 2021

Thanks, @thomasyu888! I'll copy this to an issue for reference.

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.

2 participants