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

Make gulp "processindex" command work under Windows #3359

Merged
merged 4 commits into from
Jun 23, 2017
Merged

Make gulp "processindex" command work under Windows #3359

merged 4 commits into from
Jun 23, 2017

Conversation

uglycoyote
Copy link
Contributor

ProcessIndex command was using single quotes around the index.html file
path, need double quotes to work on Windows. Tested double quotes
under Ubuntu and still worked there.

Contributor Checklist:

  • [N/A -- if someone had ever run this under windows, it would have caught the problem] I have updated the unit tests
  • [ YES ] I have created a file in the master/buildbot/newsfragment directory (and read the README.txt in that directory)
  • [N/A -- was not documented to begin with] I have updated the appropriate documentation

ProcessIndex command was using single quotes around the index.html file
path, need double quotes to work on Windows.   Tested double quotes
under Ubuntu and still worked there.
…ning how newsfragments work, since it was not clear that the newsfragments get deleted when published -- fact that the directory was nearly empty lead me to assume that news fragments were an unused feature.
@Frodox
Copy link
Member

Frodox commented Jun 22, 2017

Yup, I think processIndex and ConsoleView improvements - two different PR

@uglycoyote
Copy link
Contributor Author

@Frodox This is a new PR off a branch that I created. I created this branch off an older commit (before I started my console UI changes. I don't see any console UI changes in the 3 commits listed in this pull request. Seems like the tests start failing on the change where I added the news fragment. Not clear why.

@codecov
Copy link

codecov bot commented Jun 22, 2017

Codecov Report

Merging #3359 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3359   +/-   ##
=======================================
  Coverage   88.23%   88.23%           
=======================================
  Files         322      322           
  Lines       33441    33441           
=======================================
  Hits        29506    29506           
  Misses       3935     3935

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 3944bb0...bc77657. Read the comment docs.

@tardyp
Copy link
Member

tardyp commented Jun 23, 2017

I fixed your release note

@tardyp tardyp merged commit 6fb0d13 into buildbot:master Jun 23, 2017
@uglycoyote
Copy link
Contributor Author

I think the issue here is that it is spell-checknig my news fragment, and does not like that i used the word "processindex".

relnotes/index.rst:23:processindex:["process index", "process-index", "preprocessing", "procession", "teleprocessing", "microprocessing", "preciousness", "interprocess"]

I will try backquoting the identifier.

There's also some other test failing due to a timeout -- I don't think that has anything to do with my changes.

https://nine.buildbot.net/#/builders/10/builds/639/steps/7/logs/stdio

@uglycoyote
Copy link
Contributor Author

Oh, thanks @tardyp ! Did not see your comment from earlier!

@tardyp
Copy link
Member

tardyp commented Jun 23, 2017

eh no problem. Congratz for your first GH contribution!!

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.

3 participants