NotificationsYou must be signed in to change notification settings
Fork219
Star781

Feature/win debug dll (closes #1035)#1037

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

eddelbuettel merged 5 commits intomasterfromfeature/win_debug_dll

Dec 17, 2019

Merged

Feature/win debug dll (closes #1035)#1037

eddelbuettel merged 5 commits intomasterfromfeature/win_debug_dll

Dec 17, 2019

Conversation

Copy link

Member

eddelbuettel commentedDec 17, 2019

See the discussion and resolution in#1035 -- this addresses a narrow need for debugging when using windows andsourceCpp().

Checklist

Code compiles correctly
R CMD check still passes all tests
Document the changes by file inChangeLog
User confirmed issue as addressed

eddelbuettel added2 commits

December 15, 2019 07:44

add new option windowsDebugDLL

26befe8

roll minor version

cd8d2dd

Copy link

codecov-io commentedDec 17, 2019•
edited
Loading

Codecov Report

Merging#1037 intomaster willincrease coverage by<.01%.
The diff coverage is100%.

@@            Coverage Diff             @@##           master    #1037      +/-   ##==========================================+ Coverage   91.92%   91.92%   +<.01%==========================================  Files          64       64                Lines        2935     2936       +1     ==========================================+ Hits         2698     2699       +1  Misses        237      237

Impacted Files	Coverage Δ
R/Attributes.R	`90.4% <100%> (+0.02%)`	⬆️

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update43a6d84...9da65da. Read thecomment docs.

add a few NEWS.Rd entries from a few recent PRs

4b86c1b

coatless reviewed

Dec 17, 2019

View reviewed changes

R/Attributes.R

		}else {
		if (verbose&&windowsDebugDLL) {
		message("The 'windowsDebugDLL' toggle is ignored on non-Windows platforms.")
		windowsDebugDLL<-FALSE# now we do not need to deal with OS choice below

Copy link

Contributor

coatlessDec 17, 2019

Choose a reason for hiding this comment

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

Shouldn't this be outside the if()? As it stands now, this will only trigger ifverbose = TRUE is passed (it normally isn't.)

Copy link

MemberAuthor

eddelbuettelDec 17, 2019

Choose a reason for hiding this comment

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

Nice catch. I think you are correct. Will fix.

Copy link

MemberAuthor

eddelbuettelDec 17, 2019•
edited
Loading

Choose a reason for hiding this comment

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

New variant:

if (verbose) {                                                                                                                                                                                                         message("The 'windowsDebugDLL' toggle is ignored on""non-Windows platforms.")                                                                                                                                                                              }windowsDebugDLL<-FALSE# now we do not need to deal with OS choice below        }# #nocov end

Better...

eddelbuettel added2 commits

December 17, 2019 12:05

improved when-not-on-windows logic (thanks,@coatless)

3268980

add missing comma

9da65da

coatless reviewed

Dec 17, 2019

View reviewed changes

inst/NEWS.Rd

		\itemSafer \code{Rcpp_list}, \code{Rcpp_lang}and
		\code{Function.operator()} (Romainin \ghpr{1014}, \ghit{1015}).
		\itemAnumberof \code{#nocov} markers were added (Dirk in
		\ghprP1036}).

Copy link

Contributor

coatlessDec 17, 2019

Choose a reason for hiding this comment

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

Suggested change

	\ghprP1036}).
	\ghpr{1036}).

Copy link

MemberAuthor

eddelbuettelDec 17, 2019

Choose a reason for hiding this comment

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

Hmpf. I could swear I can a check which should spotted that. Will fix!

coatless approved these changes

Dec 17, 2019

View reviewed changes

R/Attributes.R

		if (windowsDebugDLL) {
		if (verbose) {
		message("The 'windowsDebugDLL' toggle is ignored on"
		message("The 'windowsDebugDLL' toggle is ignored on",

Copy link

Contributor

coatlessDec 17, 2019

Choose a reason for hiding this comment

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

Good catch on this one.

Copy link

MemberAuthor

eddelbuettelDec 17, 2019

Choose a reason for hiding this comment

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

Well it was a last-second edit to comply with the@jjallaire style of less than 80 columns. I find 100 or so easier and this is what I got for it :)

Copy link

MemberAuthor

eddelbuettel commentedDec 17, 2019

Thanks for the review. There isn't all that much else here, and you caught the one obvious thinko, so I think I'll merge this now.

eddelbuettel merged commitee2e102 intomaster

Dec 17, 2019

coatless reviewed

Dec 17, 2019

View reviewed changes

inst/NEWS.Rd

		\itemUnavailablepackagesreferredtoin \code{LinkingTo}arenow
		reported (Dirkin \ghpr{1027}fixing \ghit{1026}).
		\itemThe \code{sourceCpp}functioncannowcreateadebugDLLon
		Windows (Dirkin \ghpr{1037}fixing \ghit{ghit}).

Copy link

Contributor

coatlessDec 17, 2019

Choose a reason for hiding this comment

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

Also, there's\ghit{ghit}

Copy link

MemberAuthor

eddelbuettelDec 17, 2019

Choose a reason for hiding this comment

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

Bah. Something must be wrong with the water. Or the coffee. Or both.

Changed to\ghit{1035}. Thanks again.

eddelbuettel deleted the feature/win_debug_dll branch

December 20, 2019 13:43

Labels

None yet

Movatterモバイル変換

Uh oh!

Feature/win debug dll (closes #1035)#1037

Feature/win debug dll (closes #1035)#1037

Uh oh!

Conversation

eddelbuettel commentedDec 17, 2019

Checklist

Uh oh!

codecov-io commentedDec 17, 2019• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eddelbuettelDec 17, 2019• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eddelbuettel commentedDec 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commentedDec 17, 2019•
edited
Loading

eddelbuettelDec 17, 2019•
edited
Loading