Skip to content

COMP: Replace itkStaticConstMacro with constexpr in 1D FFT classes#2805

Merged
thewtex merged 1 commit intoInsightSoftwareConsortium:masterfrom
tbirdso:fix-1d-fft-imagedimension
Oct 13, 2021
Merged

COMP: Replace itkStaticConstMacro with constexpr in 1D FFT classes#2805
thewtex merged 1 commit intoInsightSoftwareConsortium:masterfrom
tbirdso:fix-1d-fft-imagedimension

Conversation

@tbirdso
Copy link
Copy Markdown
Contributor

@tbirdso tbirdso commented Oct 12, 2021

Resolves Nightly Build errors at https://open.cdash.org/viewBuildError.php?buildid=7507611

PR Checklist

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions Bot added area:Filtering Issues affecting the Filtering module type:Compiler Compiler support or related warnings labels Oct 12, 2021
@jhlegarreta
Copy link
Copy Markdown
Member

👍 for doing this.

#2800 can then be reverted I guess. My fault for not having solved the root problem.

Also, these are errors rather than warnings.

@N-Dekker
Copy link
Copy Markdown
Contributor

Off-topic but slightly related: I see two itkGetMacro calls in these files. As far as I know, itkGetConstMacro would be preferred.

@jhlegarreta
Copy link
Copy Markdown
Member

Off-topic but slightly related: I see two itkGetMacro calls in these files. As far as I know, itkGetConstMacro would be preferred.

That should clearly go in a separate PR; if I'm not mistaken it changes the API and should go into a deprecation cycle.

@seanm
Copy link
Copy Markdown
Contributor

seanm commented Oct 12, 2021

FYI, I run the RogueResearch submissions. If y'all could try to remember to CC me, it would save duplicate work of me chasing things now then finding hours later this PR exists already. :)

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 12, 2021

I guess we are subscribed to all news in ITK repository. I kind of assumed the other active members (including you @seanm) are "watching" it too.

@seanm
Copy link
Copy Markdown
Contributor

seanm commented Oct 12, 2021

I guess we are subscribed to all news in ITK repository.

I'm not, but I'll try turning that on. Sounds like it might be an intense firehose though...

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 12, 2021

If the thread ends in "@x merged #Y", you can archive the conversation without reading through. But having the notifications allows you to not miss the action.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Oct 12, 2021

You can always reduce your watching level to "mentions only", if full watching is overwhelming. The upcoming weeks are likely to have more traffic than usual, due to proximity of a release.

@seanm
Copy link
Copy Markdown
Contributor

seanm commented Oct 12, 2021

You can always reduce your watching level to "mentions only",

That's what I was set to until just now, but no one @mentioned me on this PR, which was my point. :) But now I'm on 'All Activity', will see how that goes...

@jhlegarreta
Copy link
Copy Markdown
Member

That's what I was set to until just now, but no one @mentioned me on this PR, which was my point. :) But now I'm on 'All Activity', will see how that goes...

I take the blame on this @seanm. I should have tagged you for the previous related PRs that I started and that motivated this one. Sorry.

@seanm
Copy link
Copy Markdown
Contributor

seanm commented Oct 12, 2021

I take the blame on this @seanm. I should have tagged you for the previous related PRs that I started and that motivated this one. Sorry.

Oh it's no biggie! Was just a suggestion for next time.

Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

👍

@thewtex
Copy link
Copy Markdown
Member

thewtex commented Oct 12, 2021

Thanks, all, for hunting these down!

@thewtex thewtex merged commit d1acb52 into InsightSoftwareConsortium:master Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module type:Compiler Compiler support or related warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants