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

Ci windows appveyor pr #8226

Closed
wants to merge 20 commits into from
Closed

Conversation

xavArtley
Copy link
Contributor

xav artley and others added 19 commits September 6, 2018 09:25
* Add balanced theme

* Shorten tick out

* Rename balanced to caliber and add inline plots to docstring

* Separate shows
* Added __copy__ and __deepcopy__ hooks to PropertyValueColumnData.
Modified ColumnDataSource.__init__ to make deep copies of supplied dicts.
Changed the docstring of ColumnDataSource to insert a note documenting this behaviour.

* changed ColumnDataSource to create a shallow copy of passed dicts instead.

* Added a blank line after note directive in ColumnDataSource docstring.
Changed __copy__ to just use the dict built-in instead of copy.copy

* Removed modifications from sources.py, except for note in docstring.
Added tests for ColumnDataSource to check if initializing with PropertyValueColumnData creates a shallow copy.
Addet tests for PropertyValueColumnData to check __copy__ and __deepcopy__ hooks.

* fixed missing newlines at EOF

* fixed missing newlines at EOF

* missed newline error
* don't dynamically generate bokeh.command docstring

* static docstring, 100% cov for bootstrap

* add tornado to info, 100% cov

* 100% cov for html

* factor common svg/png code, better size warning, 100% cov

* 100% cov for bokeh.util.version

* session_id coverage
* Added unit/integration tests for python3.7

* Remove python3.7 integration test
deps += '" "'.join(s for s in spec)
deps += '"'
else:
deps += '" "'.join(s for s in spec)
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't seem correct in the non-windows case. It looks like it would produce:

foo" "bar" "baz

with no quotes at the beginning. I think it needs to be deps += " ".join(...) as before.

However, is it possible to a avoid changing this script at all, and put the necessary logic to handle this output in the windows scripts? AFAIK this works on windows with the original script:

for /F "delims=" %i in ('python scripts\deps.py build run') do (conda install %i)

Copy link
Member

Choose a reason for hiding this comment

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

Tho perhaps that windows command is much slower (I think it does a separate install for each pkg?)

Copy link
Contributor Author

@xavArtley xavArtley Sep 8, 2018

Choose a reason for hiding this comment

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

On windows i never succeed to run the command of the documentation directly because of node <>=. Thats why i need to use quotes. With powershell or dos

@bryevdv
Copy link
Member

bryevdv commented Sep 7, 2018

@xavArtley is it possible to rebase on lastest master? I think this is branched from an older commit and that is why there is exrta cruft in the commit log


environment:
matrix:
# Python 3.6
Copy link
Member

Choose a reason for hiding this comment

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

I think the latest or possibly upcoming miniconda is 3.7 I would say leave the comment out since it will soon be outdated if it is not already

alt="Current AppVeyor build status" />
</a>
</td>
</tr>
Copy link
Member

Choose a reason for hiding this comment

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

We will need to make a bokeh project account and have the link here refer to that. For now can you just leave this out? I need to re-edit the README anyway, I will re-organize that table to accommodate this badge at that point.

Copy link
Member

Choose a reason for hiding this comment

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

OK we already have a bokeh project on Appveyor, linked under the "bokeh-integrations" GitHub account. If you let me know any specific configuration necessary, I can make any changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just sign in on appveyor with my github account and add the repo to projects. Now each time a modification is done its trigger a build

@xavArtley
Copy link
Contributor Author

I'll will make a cleaner pr next week. I'm now in holidays for one week andhave just a tablet

@xavArtley
Copy link
Contributor Author

I close this PR, I made a cleaner branch

@bryevdv
Copy link
Member

bryevdv commented Sep 17, 2018

OK, look forward to it, let me know if I can help with anything

@xavArtley xavArtley deleted the ci_windows_appveyor_pr branch September 27, 2018 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appveyor build and test for windows
5 participants