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

Move Sobek entirely out of the business logic #4219

Open
9 of 15 tasks
Tracked by #4236
inancgumus opened this issue Oct 12, 2023 · 0 comments
Open
9 of 15 tasks
Tracked by #4236

Move Sobek entirely out of the business logic #4219

inancgumus opened this issue Oct 12, 2023 · 0 comments

Comments

@inancgumus
Copy link
Member

inancgumus commented Oct 12, 2023

What?

Move Sobek entirely out of the business logic.

Why?

  • Prevent Sobek data races.
  • Better type-safety using native Go constructs.
  • Developer productivity increase.
  • Separation of concerns.
  • Moving Sobek handling to the mapping layer will make the code easy to reason about, reduce the business logic's Sobek dependency, and simplify the business logic code and tests.
  • Having Sobek in the business logic requires constant encoding and decoding.

Desobekifying Sobek transformation

Besides moving the calling of Sobek option parsing in the business logic, we also plan to move the Sobek-dependent option parsing part out of the business logic. Once we finish doing the same with the rest of the option parsing, we can entirely remove the Sobek dependency from the business logic.

We currently parse Sobek values into our business logic types, but the parsing dependency is in an unrelated package (common). To enhance maintainability, we move the parsing process and keep related components together (in the mapping layer). Additionally, we could implement a Validate method for the business types (in the common package) to check their validity separately from the Sobek transformation.

Moreover, we use pointers quite liberally, which is not good in terms of reduced performance and also because of many nil-pointer panics. Also, since our module is highly current, it's dangerous to have so many pointers around. So, another goal would be to get rid of as many pointer-ed options as possible to boost stability even more.

See grafana/xk6-browser#1510 for an example.

Tasks

Tasks

Preview Give feedback
  1. area: browser
  2. area: browser
  3. area: browser
  4. area: browser
  5. 0 of 2
    area: browser refactor
  6. 12 of 13
    mapping stability
    inancgumus
  7. 2 of 2
    maintenance
    inancgumus
  8. 2 of 2
    refactor stability
    inancgumus
  9. area: browser
  10. 2 of 2
    mapping stability
    inancgumus
  11. 2 of 2
    maintenance stability
    ankur22
  12. 2 of 2
    refactor stability
    ankur22
  13. 2 of 2
    refactor stability
    ankur22
  14. refactor stability
    ankur22
  15. refactor stability
    ankur22

Related PR(s)/Issue(s)

grafana/xk6-browser#271

@inancgumus inancgumus changed the title Move option parsing to the mapping layer Move option parsing into the mapping layer Oct 12, 2023
@inancgumus inancgumus changed the title Move option parsing into the mapping layer Move Goja entirely out of the business logic Jan 26, 2024
@ankur22 ankur22 pinned this issue Feb 7, 2024
@inancgumus inancgumus changed the title Move Goja entirely out of the business logic Move Sobek entirely out of the business logic Jun 7, 2024
@inancgumus inancgumus transferred this issue from grafana/xk6-browser Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants