Skip to content

Remove some useless code in fft.NewDomain#153

Merged
ThomasPiellard merged 1 commit intoConsensys:feat/fft_cleanupfrom
zhiqiangxu:opt_NewDomain
Feb 22, 2022
Merged

Remove some useless code in fft.NewDomain#153
ThomasPiellard merged 1 commit intoConsensys:feat/fft_cleanupfrom
zhiqiangxu:opt_NewDomain

Conversation

@zhiqiangxu
Copy link
Copy Markdown
Contributor

Just stumpled upon some useless code.

@ThomasPiellard
Copy link
Copy Markdown
Contributor

Hi @zhiqiangxu , yes that's right thanks for spotting this. There is also PrecomputeReversedTable which is not used. Could you modify your PR to merge it to develop instead of master?

@zhiqiangxu zhiqiangxu changed the base branch from master to develop February 22, 2022 10:24
@zhiqiangxu
Copy link
Copy Markdown
Contributor Author

zhiqiangxu commented Feb 22, 2022

Hi @zhiqiangxu , yes that's right thanks for spotting this. There is also PrecomputeReversedTable which is not used. Could you modify your PR to merge it to develop instead of master?

Done!

Do you want to remove the PrecomputeReversedTable you mentioned? It may cause compatibility problem for old version though.

@ThomasPiellard
Copy link
Copy Markdown
Contributor

ThomasPiellard commented Feb 22, 2022

Yes I will remove it, some comments need to be updated as well. Normally if I am not mistaken after the removal of PrecomputeReversedTable only the marshal functions will be affected...

@zhiqiangxu
Copy link
Copy Markdown
Contributor Author

Yes I will remove it, some comments need to be updated as well. Normally if I am not mistaken after the removal of PrecomputeReversedTable only the marshal functions will be affected...

Right.

@ThomasPiellard
Copy link
Copy Markdown
Contributor

I didn't pay attention but in fact you didn't modify the templates for the code generation so there is some additional code to write. To save your commit, could you tweak the PR again to merge it to feat/fft_cleanup (I juste created this branch). I can add the other required modifications right now.

@zhiqiangxu zhiqiangxu changed the base branch from develop to feat/fft_cleanup February 22, 2022 13:36
@zhiqiangxu
Copy link
Copy Markdown
Contributor Author

I didn't pay attention but in fact you didn't modify the templates for the code generation so there is some additional code to write. To save your commit, could you tweak the PR again to merge it to feat/fft_cleanup (I juste created this branch). I can add the other required modifications right now.

Done!

@ThomasPiellard ThomasPiellard merged commit d04d227 into Consensys:feat/fft_cleanup Feb 22, 2022
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