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

[PLATFORM-2230]: Create an MVP for webkit support #1

Merged
merged 22 commits into from
Oct 4, 2024

Conversation

MaeIsBad
Copy link
Member

https://prima-assicurazioni-spa.myjetbrains.com/youtrack/issue/PLATFORM-2230

this is very unfinished and could use a bunch of refactors but it's good enough for an mvp

@MaeIsBad MaeIsBad requested a review from a team September 24, 2024 13:50
Copy link

@cpiemontese cpiemontese left a comment

Choose a reason for hiding this comment

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

I can't comment that much on the Rust code itself, the question I have is with maintenability

  • We have Nix, which not many people in Prima are familiar with
  • We have "complex" Rust code

The second thing could probably be improved with more documentation, the first one I don't know... I don't want to kill the Nix dream but it is something we need to consider

That said, really nice job and really clean code 😄

src/lib.rs Outdated Show resolved Hide resolved
@MaeIsBad
Copy link
Member Author

We have Nix, which not many people in Prima are familiar with

yeahhh, but the alternative for our use case was to make a debian package, which I don't know how to do myself, and I don't think many more people at prima know how to.

We have "complex" Rust code

yeahhhh, this is interacting with GTK so not much can be done in terms of fixing it

Copy link
Member

@cottinisimone cottinisimone left a comment

Choose a reason for hiding this comment

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

For the code part it seems ok to me. I didn't even took a look at the nix part :)

Copy link

@cpiemontese cpiemontese left a comment

Choose a reason for hiding this comment

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

I think it's fine for an MVP, if the Nix or Rust code can be documented better let's do that otherwise it's ok

@MaeIsBad MaeIsBad merged commit f34d635 into master Oct 4, 2024
19 checks passed
@MaeIsBad MaeIsBad deleted the PLATFORM-2230/task/create-an-mvp-for-webkit-support branch October 4, 2024 09:11
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.

3 participants