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

Generate html and json of supported css properties. #10208

Merged
merged 5 commits into from Mar 29, 2016

Conversation

@jrasanen
Copy link
Contributor

jrasanen commented Mar 26, 2016

Fixes #10196. Outputs html and json of supported css properties to target/doc/ directory when deploying github-pages.


This change is Reviewable

@highfive
Copy link

highfive commented Mar 26, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/list_properties.py, components/style/properties.html.mako
  • @SimonSapin: components/style/list_properties.py, components/style/properties.html.mako
@highfive
Copy link

highfive commented Mar 26, 2016

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
</section>

<script src="../jquery.js"></script>
<script>

This comment has been minimized.

@SimonSapin

SimonSapin Mar 26, 2016

Member

Since this is a Mako template that has properties available, could Mako generate the HTML table directly, without JavaScript?

This comment has been minimized.

@jrasanen

jrasanen Mar 26, 2016

Author Contributor

@SimonSapin Hi! Fixed the template to use directly Mako instead of Javascript + JSON.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Mar 27, 2016

Reviewed 1 of 3 files at r1.
Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


components/style/list_properties.py, line 37 [r2] (raw file):
Would it be possible to restructure this so that we don't edit list_properties.py, but instead in upload_docs.sh:

  1. Redirect the output of this script to a file (e.g., css-properties.json, as you named it here)

  2. Run the mako stuff, possibly in another small python script that lives in the etc/ci directory, if that is required.


Comments from the review on Reviewable.io

@jrasanen
Copy link
Contributor Author

jrasanen commented Mar 27, 2016

Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


components/style/list_properties.py, line 37 [r2] (raw file):
Yes, agreed, having a script which just outputs the JSON is better since then its more generic. Also makes it easier just to pipe the output and change the end location of it in CI script.

My initial implementation was just a plain html reading the json with jquery (which seems to be included with the documentation). I could either do that again or do a python/shell script which generates the property table?


Comments from the review on Reviewable.io

@jrasanen
Copy link
Contributor Author

jrasanen commented Mar 27, 2016

Review status: 1 of 3 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


components/style/list_properties.py, line 37 [r2] (raw file):
I think I got Simon Sapin's suggestion on Github wrong regarding generating the static html in that script. Sorry about that!


Comments from the review on Reviewable.io

@SimonSapin
Copy link
Member

SimonSapin commented Mar 28, 2016

@bors-servo r+


Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


components/style/list_properties.py, line 37 [r2] (raw file):
What you have in this PR now is what I had in mind. Thanks!


Comments from the review on Reviewable.io

@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

📌 Commit b87d6e7 has been approved by SimonSapin

@SimonSapin
Copy link
Member

SimonSapin commented Mar 28, 2016

@bors-servo r-

There’s an error on Travis-CI:

0.37s$ ./mach test-unit

Traceback (most recent call last):

  File "/home/travis/build/servo/servo/components/style/list_properties.py", line 32, in <module>

    with open(os.path.join(servo_doc_path, 'css-properties.json'), "w") as out_file:

IOError: [Errno 2] No such file or directory: '/home/travis/build/servo/servo/target/doc/servo/css-properties.json'

Please change the script so that it creates directories as needed if target does not exist. (Try using https://git-scm.com/docs/git-worktree if you have a build you don’t want to remove.)

jrasanen added 3 commits Mar 26, 2016
Outputs html and json file of supported css properties to target/doc/
directory when deploying github-pages.
@jrasanen jrasanen force-pushed the jrasanen:jr/issue10196 branch from b87d6e7 to 8e2af4c Mar 28, 2016
@jrasanen
Copy link
Contributor Author

jrasanen commented Mar 28, 2016

@SimonSapin good catch, modified script to create the directory and added the json printing back, I think ./mach test-unit needs it too. Ran those tests locally too and it passed now.

@SimonSapin
Copy link
Member

SimonSapin commented Mar 28, 2016

Printing would make the CI logs for upload_docs.sh noisy. Could you rather change test-unit to read the JSON file after running the command?

@jrasanen
Copy link
Contributor Author

jrasanen commented Mar 28, 2016

@SimonSapin, modified mach test unit to run the script to generate JSON and read it afterwards

@SimonSapin
Copy link
Member

SimonSapin commented Mar 28, 2016

Thanks!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

📌 Commit 6de7228 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

Testing commit 6de7228 with merge b8ef3f9...

bors-servo added a commit that referenced this pull request Mar 28, 2016
Generate html and json of supported css properties.

Fixes #10196. Outputs html and json of supported css properties to `target/doc/` directory when deploying github-pages.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10208)
<!-- Reviewable:end -->
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Mar 28, 2016

Huh. New intermittent?

Tests with unexpected results:
  ▶ TIMEOUT [expected FAIL] /css21_dev/html4/clear-applies-to-008.htm
  │ 
  │ Shutting down the Constellation after generating an output file or exit flag specified
  │ thread 'Constellation' panicked at 'called `Result::unwrap()` on an `Err` value: IoError(Error { repr: Os { code: 104, message: "Connection reset by peer" } })', ../src/libcore/result.rs:746
  │ stack backtrace:
  │    1:     0x7f56144d7a40 - sys::backtrace::tracing::imp::write::h495bc662e480d01f2cv
  │    2:     0x7f56144dc89f - panicking::default_handler::_$u7b$$u7b$closure$u7d$$u7d$::closure.44522
  │    3:     0x7f56144dc518 - panicking::default_handler::h4e73712d1e927dfeH0z
  │    4:     0x7f56144c67dc - sys_common::unwind::begin_unwind_inner::h0e1be209f5e9e87dg2t
  │    5:     0x7f56144c7788 - sys_common::unwind::begin_unwind_fmt::h45dc2fac9b5e0d23m1t
  │    6:     0x7f56144d6d41 - rust_begin_unwind
  │    7:     0x7f56145142bf - panicking::panic_fmt::h40f5ec0cdc3fc429FRL
  │    8:     0x7f561382523e - result::unwrap_failed::h9898011520364998313
  │    9:     0x7f561384e389 - font_cache_thread::FontCacheThread::exit::ha2d0eb6658e647b1uMn
  │   10:     0x7f5612b3fa35 - constellation::Constellation<LTF, STF>::handle_exit::h10693835640492743256
  │   11:     0x7f5612b1cce6 - constellation::Constellation<LTF, STF>::handle_request::h6557854063750769946
  │   12:     0x7f5612b10f4c - sys_common::unwind::try::try_fn::h5641394920290239990
  │   13:     0x7f56144d6ccb - __rust_try
  │   14:     0x7f56144d6c5d - sys_common::unwind::inner_try::he9b0b11c6b5b4e7fiZt
  │   15:     0x7f5612b1240a - boxed::F.FnBox<A>::call_box::h4738823920327326506
  │   16:     0x7f56144dae69 - sys::thread::Thread::new::thread_start::hf1ffcc04a41608eb9Xy
  │   17:     0x7f561076c181 - start_thread
  │   18:     0x7f561028347c - __clone
  └   19:                0x0 - <unknown>

program finished with exit code 1
elapsedTime=640.913206
@jdm
Copy link
Member

jdm commented Mar 28, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

Testing commit 6de7228 with merge d910ae1...

bors-servo added a commit that referenced this pull request Mar 28, 2016
Generate html and json of supported css properties.

Fixes #10196. Outputs html and json of supported css properties to `target/doc/` directory when deploying github-pages.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

💔 Test failed - status-appveyor

@jdm
Copy link
Member

jdm commented Mar 28, 2016

@bors-servo: retry

  • appveyor weirdness
@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

💣 Failed to start rebuilding: Unknown error

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

Testing commit 6de7228 with merge 365f7e2...

bors-servo added a commit that referenced this pull request Mar 29, 2016
Generate html and json of supported css properties.

Fixes #10196. Outputs html and json of supported css properties to `target/doc/` directory when deploying github-pages.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

💔 Test failed - status-appveyor

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Mar 29, 2016

@bors-servo retry

not sure what is going on here - it looks like the appveyor build is kicking off either before or after auto has moved to a change that does not contain this commit:

git clone -q --branch=auto https://github.com/servo/servo.git C:\projects\servo
git checkout -qf 365f7e2f685ce7d21bd37af1b9620467908f582f
fatal: reference is not a tree: 365f7e2f685ce7d21bd37af1b9620467908f582f
Command exited with code 128
@jrasanen
Copy link
Contributor Author

jrasanen commented Mar 29, 2016

@larsbergstrom maybe squashing the commits & pushing would fix it?

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

Testing commit 6de7228 with merge df73a18...

bors-servo added a commit that referenced this pull request Mar 29, 2016
Generate html and json of supported css properties.

Fixes #10196. Outputs html and json of supported css properties to `target/doc/` directory when deploying github-pages.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10208)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2016

@bors-servo bors-servo merged commit 6de7228 into servo:master Mar 29, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@jrasanen jrasanen deleted the jrasanen:jr/issue10196 branch Mar 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.