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.

Sign up for GitHub

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

Jump to bottom

Style: Extract is redundant #389

Closed
Aloento opened this issue Aug 9, 2022 · 6 comments
Closed

Style: Extract is redundant #389

Aloento opened this issue Aug 9, 2022 · 6 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Aloento
Copy link
Member

Aloento commented Aug 9, 2022

According to the results of yesterday's discussion, we will merge the Extract operation into the main method call.
@Aloento Aloento added the enhancement New feature or request label Aug 9, 2022
@Aloento Aloento self-assigned this Aug 9, 2022
@outcatcher outcatcher added the documentation Improvements or additions to documentation label Aug 9, 2022
@outcatcher
Copy link
Contributor

Contribution guide should be update accordingly in #390

@Aloento
Copy link
Member Author

Aloento commented Aug 9, 2022

@outcatcher This is not all documentation-related issues; this issue also includes longer-cycle refactoring tasks.

@outcatcher
Copy link
Contributor

outcatcher commented Aug 9, 2022 •
edited
Loading

But it also includes documentation. In most cases labels are used (at least, were used) in "including smth" way. As it includes documentation changes too, in can be marked with that label 🙂

@outcatcher
Copy link
Contributor

Extract methods should be replaced: ExtractIntoSlicePtr and ExtractIntoStructPtr should be make functions.
I would suggest to do that before merging of #381 #384. Other way you have to use workarounds.

@Aloento
Copy link
Member Author

Aloento commented Aug 9, 2022

@outcatcher
This is not realistic at the moment, as the methods related to ExtractInto are all under the Result structure.
My idea is to do the first round of refactoring and remove Extract. After that, we will do a second round of refactoring to remove the Result structure completely.

If we want to do this task in one go, then we have to do another independent branch. This will seriously delay the release.
Using progressive, it is also possible to make simultaneous changes to the TF Provider at the same time.

@Aloento Aloento mentioned this issue Aug 9, 2022
Merged
@outcatcher outcatcher mentioned this issue Aug 10, 2022
Merged
otc-zuul bot pushed a commit that referenced this issue Aug 10, 2022
@outcatcher
[Refactor] Introduce extract functions (#391)
e92c6cf 
[Refactor] Introduce extract functions

What this PR does / why we need it
Using extract functions allows to avoid using Result
structure and its derivatives
Which issue this PR fixes

Part of #389
Special notes for your reviewer
This is what I was talking about in comments of #389 and in #381 and #384 discussions
Also, if unit tests are required, I will provide them later today

Reviewed-by: Artem Lifshits <None>
Reviewed-by: Aloento <None>
Reviewed-by: Anton Sidelnikov <None>
@Aloento Aloento removed their assignment Jul 23, 2024
@anton-sidelnikov
Copy link
Member

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@outcatcher @Aloento @anton-sidelnikov