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

Initial pyjade port to pypugjs #5298

Merged
merged 2 commits into from May 29, 2020
Merged

Initial pyjade port to pypugjs #5298

merged 2 commits into from May 29, 2020

Conversation

dol-sen
Copy link
Contributor

@dol-sen dol-sen commented May 13, 2020

Signed-off-by: Brian Dolbec dolsen@gentoo.org

This just swaps out the use of pyjade code for a maintained fork of the original pyjade code.
I have a paired down version of this patch which applies to the 2.7.0 release for our Gentoo package repository. I have tested buildbot both with the test suite and running the example buildbot "Hello world" master.cfg

Signed-off-by: Brian Dolbec <dolsen@gentoo.org>
@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #5298 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5298   +/-   ##
=======================================
  Coverage   89.28%   89.28%           
=======================================
  Files         335      335           
  Lines       36526    36526           
=======================================
  Hits        32612    32612           
  Misses       3914     3914           
Impacted Files Coverage Δ
master/buildbot/www/config.py 95.78% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8920cec...3c879d7. Read the comment docs.

@@ -131,6 +131,8 @@ This server is configured with the ``www`` configuration key, which specifies a
* quotes in attributes are not quoted. https://github.com/syrusakbary/pyjade/issues/132
Copy link
Member

Choose a reason for hiding this comment

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

we should probably make a quick test and see if this is still the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, the new code is identical aside from some minor reformatting. So, no it has not been fixed.

The new code is at lines 145 to 155 of nodes.py

@tardyp
Copy link
Member

tardyp commented May 15, 2020

Looks good. If we can verify that pyjade/132 is fixed that would actually make a nice improvement. We will need a news fragment as the upgrade will not be seemless as an optional dependency is changing.

@dol-sen
Copy link
Contributor Author

dol-sen commented May 15, 2020

I've opened this issue with pypugjs:
Double quotes in attribute (carry-over bug from pyjade issue #132) #60
at kakulukia/pypugjs#60

Hopefully it can be fixed quickly. Have you got an up-to-date link to a file that this fails for? I can take a look at fixing the code to submit a PR.

@p12tic
Copy link
Member

p12tic commented May 26, 2020

@dol-sen Could you add a newsfragment? I would then merge the PR. Thanks a lot!

@dol-sen
Copy link
Contributor Author

dol-sen commented May 26, 2020

Did tardyp get that bug fixed? We did confirm the bug still exists in pyjade. He was working on a fix.

@p12tic
Copy link
Member

p12tic commented May 26, 2020

There were some issues with the investigation uncovered later. See this comment: kakulukia/pypugjs#61 (comment)

In any case, this PR does not look to make matters worse, thus can be merged as is.

The pypugjs/pyjade bug can be fixed later.

@tardyp tardyp merged commit 878cca5 into buildbot:master May 29, 2020
svenstaro pushed a commit to archlinux/svntogit-community that referenced this pull request Jul 22, 2020
Ref: buildbot/buildbot#5298



git-svn-id: file:///srv/repos/svn-community/svn@637396 9fca08f4-af9d-4005-b8df-a31f2cc04f65
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.

None yet

3 participants