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

Ignore css parse errors and allow more versatile parsing implementations #108

Merged
merged 5 commits into from
Aug 13, 2022

Conversation

brbog
Copy link

@brbog brbog commented Aug 13, 2022

No description provided.

Bram Bogaert added 5 commits August 13, 2022 02:23
improve readability as to when the parsing really happens,
deprecate parse()-methods with url passed on separate from the Page-object (in which it also resides)
… inside css content

(having the css content with urls pointing to missing locations is preferable over not having it at all)
@brbog
Copy link
Author

brbog commented Aug 13, 2022

Made sure backwards compatibility is ok.

When the timing is right, another improvement could be to group all classes under the parser-package depending on the content they act on, so introducing packages ...parser.html, ...parser.css, ...parser.text, and ...parser.binary.

Changing the different parsers to "look" more like the way parsing is initiated for html content would also be nice, but probably not so great for backwards compatibility and very little gains other than more consequent code flows.

@rzo1
Copy link
Collaborator

rzo1 commented Aug 13, 2022

Thx for the PR. We can do a 4.x release and move to 5.x with breaking changes. Wdyt?

@rzo1 rzo1 merged commit b77a875 into HHN:master Aug 13, 2022
@brbog
Copy link
Author

brbog commented Aug 13, 2022

@rzo1:
A 5.x with a cleanup of all the deprecated methods sounds good. There are parts that could be refactored for the sake of consequent coding, but I would leave that as it is. The framework works, and the code is easy enough to follow.

At the moment the only things left on my list are (in this order):

  1. cleanup of deprecated methods + maybe some grouping of different parser codes in different packages (html, css, text, binary)
  2. add a little bit of documentation, because in 6 months that will really make a difference :-)
  3. migrate to JUnit5
  4. get rid of Groovy/Spock (longer term goal)

Point 1 and 2 are quick wins, not too much work. Only for 2 you would have to proofread and maybe elaborate on the frontiers part (I wrote the changes until now down, some "marketing effort" is warranted at this point I think). I believe you have experience with previous maintainer/contributors, so you are better placed to check documentation updates are correct and not offensive to them or anything. I was also thinking of making a high level design diagram to quickly have an overview of different parts in the code where one might want to extend. Do you know of/have preference for a tool?

Point 3 shouldn't be hard either, I did this for very old code of myself last year. The reason it actually became a point on the agenda is that I noticed yesterday that the JUnit5 tests I committed don't break the Maven build if you introduce errors in them on purpose.

Point 4 depends on your preference :-). I'm not a big believer in polyglot programming, but it takes time for history to prove either side right or wrong :-). Timing: I think I'll be elaborating my knowledge of WireMock within the next 6 months to a year (needed to make the migration).

@brbog brbog deleted the ignore-css-parse-errors branch August 13, 2022 13:33
@rzo1 rzo1 added this to the v4.10.1 milestone Aug 15, 2022
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.

2 participants