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

Conflation of Pilot3 Issues #95, #135, #137, #138, and #139 per the discussion in the Issue #135 PR thread #140

Merged
merged 7 commits into from
Jan 22, 2024

Conversation

robertdevine
Copy link
Collaborator

@robertdevine robertdevine commented Jan 17, 2024

Per the Issue #135 PR discussion, tidying to capture (i)efficiencies by linking the R Consortium WG Meeting Minutes starting with the October 2023 FDA Feedback with Planned Scheduling of the FDA Review Period and (ii)Pilot3 Team compilation of FDA Pilot3 Submission Feedback including the Pilot3 Team Issue Interpretation(s) as the FDA Review continues.

Looks like lints fail during checks. I'll take a look. I think we saw this lints issue before. Thx.

image

Looks like a couple of stray chars, getting late, tomorrow I'll tidy a bit more and update the PR. Thx.

image

image

@bms63 and @laxamanaj - have it down to a single lintr error otherwise looks good - the Pilot3 URL in pkgdown yaml is not passing lint - we'll get it...I'll take a look.

image

…sion thread in the Issue 135 PR. Applying some simple tidying to capture (i)efficiencies by linking the R Consortium WG Meeting Minutes starting with the October 2023 FDA Feedback with Planned Scheduling of the FDA Review Period and (ii)Pilot3 Team compilation of FDA Pilot3 Submission Feedback including the Pilot3 Team Issue Interpretation(s) as the FDA Review continues.
@robertdevine robertdevine linked an issue Jan 17, 2024 that may be closed by this pull request
@robertdevine robertdevine self-assigned this Jan 17, 2024
@bms63
Copy link
Collaborator

bms63 commented Jan 18, 2024

Hi @robertdevine - I was trying to fix the lintr issue when updating the pkg yml.

I think the headline ("Feedback from the FDA") of the menu might need ot be updated if we are listing out the minutes for the meetings as well as specific feedback given to the FDA.

Perhaps "Working Group Minutes and Regulatory Feedback" or "WG Minutes and Regulatory Feedback"?

image

@bms63
Copy link
Collaborator

bms63 commented Jan 18, 2024

I managed to reduce the number of lintr issues. We can put in nolints to deal with the commented out code sections.

Thanks @bms63, the update to the headline ("Feedback from the FDA") and "What's on the website section?" will be "WG Minutes and Regulatory Feedback".

Robert Devine added 2 commits January 18, 2024 10:22
…latory Feedback, (ii)same for Pilot3 website What's on the Website? home page section, and (iii)tidy trailing spaces identified by lintr.
…rs around comments. GH lintr recommends removal of commented out code in the Details section of the GH Action. Pilot3 Team discussed the relevance of retaining the commented out code in the WG repository. Pilot3 Team Leads suggested using nolints for commented out code. Commented code lint disabled.
@robertdevine
Copy link
Collaborator Author

@bms63 and @laxamanaj - good to go on it. PR and Issue #140 pass all checks. This should address all of the discussion points following PR #135. Should be all set for the February 2024 WG General Meeting and further feedback from FDA. Thanks. again

.lintr Outdated
@@ -1,2 +1,2 @@
linters: linters_with_defaults(line_length_linter(150), object_usage_linter=NULL, cyclocomp_linter(complexity_limit = 20))
exclusions: list("R/data.R")
linters: linters_with_defaults(line_length_linter(150), object_usage_linter=NULL, cyclocomp_linter(complexity_limit = 20), commented_code_linter = NULL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be better to just use nolint around the sections we don't want the lintr to check rather than disabling this feature.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bms63, quite a few code blocks in Pilot3 with commented codes and 'unexpected chars' to apply nolints to address this item in a 'by section' manner where we don't want lintr to check. I'll chip away at it using the same branch and PR. thx.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ummm.. i wonder if there is a way to globally set to check after line 20 or something??

Copy link
Collaborator

Choose a reason for hiding this comment

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

i don't see anything - lets just leave as it. I didn't realize we would need so many of them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, there is @bms63 Please see here.

What do you say, we should apply lintr > line 20. Let's give it a try and re-enable the commented_code_linter. thx.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh way cool!! yeah I think we shouldn't have commented out code in our scripts, but since we have program headers we should tell the lintr not to check them . maybe set it to whatever the biggest header is?

README.md Outdated
@@ -19,7 +19,7 @@ The website is divide into 4 sections that will walk you through our efforts:
1. **Project Background/Setup** - Scope of work, what we used and how we used it.
1. **Deliverables** - What was sent to the FDA - Cover Letter, TLFs, Datasets, ADRG.
1. **Conferences** - Proceedings from different conferences on Pilot Submissions
1. **FDA Feedback** - Specific feedback from FDA on this Submission
1. **WG Minutes and Regulatory Feedback** - Specific feedback from FDA on this Submission
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we say something about there being two sections one on the minutes on the other on specific feedback from FDA?

Copy link
Collaborator Author

@robertdevine robertdevine Jan 18, 2024

Choose a reason for hiding this comment

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

@bms63, will do. Same branch and PR as well. The updated verbiage in the _What's on the Site? paragraph cites two sections under the WG Minutes and Regulatory Feedback menu, Section 1: Provides links to the monthly WG Meeting Minutes, Section2: Provides links to the Pilot3 Team Compilation of the Regulatory Review and the Pilot3 Team Interpretation(s) of the regulatory feedback as the Review Process continues. thx.

Copy link
Collaborator

@bms63 bms63 left a comment

Choose a reason for hiding this comment

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

few thoughts - thanks for getting this going!!

…the Pilot3 website home page What's on the site? section WG Minutes and Regulatory Feedback menu noting two sections [links to monthly WG Meeting Minutes, link to Pilot3 Team Compilation and Interpretation(s) of the specific feedback from FDA as the Regulatory Review continues, and (iii)redacted commented code sections from the report-tlf-pilot3.Rmd source file. The redacted sections of commented out code should have little impact to reviewers.
@robertdevine
Copy link
Collaborator Author

robertdevine commented Jan 19, 2024

Thanks @bms63 and @laxamanaj. Should be all set. Further tidying, (i)re-enabled the commented_code_linter, (ii)updated the Pilot3 website home page What's on the Website? paragraph WG Minutes and Regulatory Feedback menu noting two sections [links to monthly WG Meeting Minutes, link to Pilot3 Team Compilation and Interpretation(s) of the specific feedback from FDA as the Regulatory Review continues], and (iii)redacted commented code sections from the report-tlf-pilot3.Rmd source file. The redacted sections of commented out code should have little impact to reviewers. Thx.
[Tested lintr and static analysis in posit cloud and on R Studio VM and bare metal.]

@bms63 bms63 merged commit a41548f into main Jan 22, 2024
5 checks passed
@bms63 bms63 deleted the 139-update-website-feedback-from-fda-from-october-meeting branch January 22, 2024 14:49
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.

Update Website: feedback from FDA from October Meeting
2 participants