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

Developers Read Me file added #483

Closed
wants to merge 5 commits into from

Conversation

niteshbalakrishnan
Copy link
Collaborator

Before you do a pull request, you should always file an issue and make sure the package maintainer agrees that it's a problem, and is happy with your basic proposal for fixing it. We don't want you to spend a bunch of time on something that we don't think is a good idea.

Additional requirements for pull requests:

  • Adhere to the Developer Guidelines as well as the OHDSI Code Style.

  • If possible, add unit tests for new functionality you add.

  • Restrict your pull request to solving the issue at hand. Do not try to 'improve' parts of the code that are not related to the issue. If you feel other parts of the code need better organization, create a separate issue for that.

  • Make sure you pass R check without errors and warnings before submitting.

  • Always target the develop branch, and make sure you are up-to-date with the develop branch.

Copy link
Collaborator

@katy-sadowski katy-sadowski left a comment

Choose a reason for hiding this comment

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

Just a few formatting comments. Also please change the target branch to OHDSI:develop instead of main. You may also replace the default text in the PR description with a note describing the changes in your PR :) thanks!!!


c.Using the CDMConnector package:

i. Download a sample OMOP CDM into a DuckDB database, as documented here
Copy link
Collaborator

Choose a reason for hiding this comment

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

this formatting got messed up; it's rendering as code instead of sub-bullets. also make sure to add in the hyperlinks out to CDMConnector docs on each line.


b. Create a new database in localhost for your test CDM, and create a schema in that database for the CDM tables

c.Using the CDMConnector package:
Copy link
Collaborator

Choose a reason for hiding this comment

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

add space between c. and the text


i. Download a sample OMOP CDM into a DuckDB database, as documented here
ii. Copy the CDM into your local Postgres database, as documented here
3.Fork the DataQualityDashboard repo
Copy link
Collaborator

Choose a reason for hiding this comment

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

add space between 3. and the text

ii. Copy the CDM into your local Postgres database, as documented here
3.Fork the DataQualityDashboard repo

4.Clone your fork to your computer
Copy link
Collaborator

Choose a reason for hiding this comment

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

add space between 4. and the text

@katy-sadowski
Copy link
Collaborator

closing in favor of #488

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (e18c308) 85.95% compared to head (f184df5) 85.47%.

❗ Current head f184df5 differs from pull request most recent head e49eaea. Consider uploading reports for the commit e49eaea to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   85.95%   85.47%   -0.48%     
==========================================
  Files          15       15              
  Lines         847      847              
==========================================
- Hits          728      724       -4     
- Misses        119      123       +4     

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@katy-sadowski
Copy link
Collaborator

replaced by #488

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