Skip to content

Remove gen-test script#141

Merged
alice-i-cecile merged 2 commits intoDioxusLabs:mainfrom
alice-i-cecile:gen-test-yeet
Jun 10, 2022
Merged

Remove gen-test script#141
alice-i-cecile merged 2 commits intoDioxusLabs:mainfrom
alice-i-cecile:gen-test-yeet

Conversation

@alice-i-cecile
Copy link
Copy Markdown
Collaborator

@alice-i-cecile alice-i-cecile commented Jun 10, 2022

Objective

Initial progress towards #133. We do not intend to fix this script, so it's better to remove it proactively.

See discussion in the linked issue for why these generated tests suck.

@alice-i-cecile alice-i-cecile added the code quality Make the code cleaner or prettier. label Jun 10, 2022
@alice-i-cecile alice-i-cecile enabled auto-merge (squash) June 10, 2022 19:49
@alice-i-cecile alice-i-cecile merged commit 2281228 into DioxusLabs:main Jun 10, 2022
@mockersf
Copy link
Copy Markdown
Contributor

mockersf commented Jun 10, 2022

I really don't think this is a good idea, and it pretty much removes any trust that I would have in evolutions of this library

@TimJentzsch
Copy link
Copy Markdown
Collaborator

TimJentzsch commented Jun 10, 2022

@mockersf Do you have ideas to make the generated tests easier to maintain?
Currently, they are definitely a big maintenance burden, especially for refactorings.

Maybe one solution would be to not commit them to the repository and generate them "fresh" in CI?
But I'm not sure how much time and resources that would need.

@alice-i-cecile
Copy link
Copy Markdown
Collaborator Author

alice-i-cecile commented Jun 10, 2022 •
edited
Loading

The gen-tests script itself is very broken, and currently does nothing. Removing this folder in this PR has no effect other than to clarify this state to contributors and remove unactionable errors in IDEs.

If we decide to take an alternative path in #133, we can revert this change and tackle a cleanup.

I want to synchronize with Chrome still, but I think that this code generation approach is unmaintainable.

@alice-i-cecile alice-i-cecile mentioned this pull request Jun 10, 2022
Closed
@mockersf mockersf mentioned this pull request Jun 11, 2022
Merged
jkelleyrtp pushed a commit that referenced this pull request Oct 10, 2022
@alice-i-cecile
Remove gen-test script (#141)
26191c9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

code quality Make the code cleaner or prettier.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alice-i-cecile @mockersf @TimJentzsch