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

Stabilize POC branch for split UPE project after merging the latest changes from develop #5109

Closed
timur27 opened this issue Nov 11, 2022 · 6 comments
Assignees
Labels
category: projects For any issues which are part of any project, including bugs, enhancements, etc. component: upe

Comments

@timur27
Copy link
Contributor

timur27 commented Nov 11, 2022

Description

We have the POC branch for all the split UPE work, which is a long-lived feature branch and was not merged into the development yet.

Since the last merge (develop -> poc) took place a few weeks ago, and in the meantime, there were some significant changes introduced to the payment gateways management, it was decided to merge the develop branch into poc/upe-instances-multiplied and align the split UPE implementation with the latest refactor.

Acceptance criteria

  • The latest state of develop branch is merged into poc/upe-instances-multiplied
  • Conflicts are resolved
  • Split UPE payments work as expected
  • Tests are fixed
@timur27 timur27 self-assigned this Nov 11, 2022
@timur27 timur27 changed the title Stabilize POC branch for split UPE project after latest changes on develop Stabilize POC branch for split UPE project after merging the latest changes from develop Nov 11, 2022
@timur27 timur27 added the category: projects For any issues which are part of any project, including bugs, enhancements, etc. label Nov 11, 2022
@timur27
Copy link
Contributor Author

timur27 commented Nov 16, 2022

Hey team! Please add your planning poker estimate with Zenhub @mdmoore @FangedParakeet

@FangedParakeet
Copy link
Contributor

I voted 1, because I think you've basically wrapped this one up already, @timur27, no? 😅

@timur27
Copy link
Contributor Author

timur27 commented Nov 17, 2022

I think you've basically wrapped this one up already

You're absolutely right, @FangedParakeet! The purpose of this issue is to reflect the efforts from here. I agree that it might seem too late to estimate something already completed. However, due to the discovered complexity, it was decided to create this issue and reflect the complexity of that merge.

Based on our previous discussions, creating a new issue for each develop<->poc/upe-instances-multiplied alignment doesn't make much sense. Instead, I understand that we create issues only for complex cases, and the current issue is a perfect example. Please let me know if I wasn’t clear or missed anything.

@timur27 timur27 closed this as completed Nov 17, 2022
@FangedParakeet
Copy link
Contributor

Instead, I understand that we create issues only for complex cases, and the current issue is a perfect example.

Yeah, exactly! The purpose of popping out issues and purposefully pointing is predominantly to help our planning. That way anyone can look at the issues in the current sprint--or more broadly the epic as a whole--and estimate the amount of work that we still have ahead of us.

I think this issue was a great example of that, because it was a merge, but a task that required some considerable effort and a few reviews to boot, so it had a noticeable lifetime from the moment it was conceived. If there are any tasks that have already been completed or that will be completed by the time that they're noticed, I don't think it's so important to ensure that they are all pointed out.

The only real reason to retroactively point issues would be to capture some effort that could be measured in some future analysis. I didn't really think about this latter case at all until I just typed that previous sentence a few moments ago, so let me know if you think that's important and I'll bump up the points on this issue, so that it proves that you did a good bit of work here. 😀

@timur27 timur27 reopened this Nov 18, 2022
@timur27
Copy link
Contributor Author

timur27 commented Nov 18, 2022

From my perspective, I gave a 3 SP here to reflect all the efforts made within this task. Among others, there were:

  • The merge itself with conflict resolving (I would give it a 0.5SP if we would have such an estimate possible, but as we don't, I am giving it a 0)
  • Minor fixes without any logic change (this, this and this. The last required some real effort, especially debugging this change to make the checkout work after the merge since UPE payment methods were not rendered - estimating all these efforts for 1 SP.
  • Finding out what's wrong with tests and connecting the changes both from LINK payment method addition (which took place on develop in the meantime) and token behavior corrections for SEPA - giving it another 1 as I needed to align the LINK token handling with the SEPA token changes in order to make tests pass, as both changes came in parallel.
  • Adapt gateways initialization for the newly created checkout gateways and find ways to make sure that WC_Payments_UPE_Checkout supports multiple gateways, instead of one $card_gateway. From my perspective, this is a 1 SP. Despite being a tiny change in the end, this step itself was the most time consuming one for me, as I was still discovering split UPE and analyzing the solution design which was implemented during the last few months.

My goal was not to give a 3SP because there are three steps each for 1 SP. According to some Scrum experts, the final story points estimation should not be the sum of all the steps to accomplish the task. :) This is why I rather tried to go through all the steps above and think about the overall estimation, which eventually brought me to 3 SP.

I think this issue also required an apparent effort for you to test all the flows and confirm if they work (thanks to this, a few problems to fix were found, which I appreciate).

I am happy to hear (actually read) other opinions if you have other visions of the effort needed here. @FangedParakeet @mdmoore

The issue was reopened as it's the only state where the estimation is possible.

@FangedParakeet
Copy link
Contributor

So shall it be written, so shall it be done. 👍 Just locking it down, since Mike is AFK and now we can just close this one out again. Sorry for being so quick to judge the first time!

@timur27 timur27 closed this as completed Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: projects For any issues which are part of any project, including bugs, enhancements, etc. component: upe
Projects
None yet
Development

No branches or pull requests

2 participants