Skip to content

MAINT: Let CircleCI render PRs to improve PR review#85

Merged
effigies merged 6 commits intobids-standard:gh-pagesfrom
sappelhoff:circle
Jan 21, 2020
Merged

MAINT: Let CircleCI render PRs to improve PR review#85
effigies merged 6 commits intobids-standard:gh-pagesfrom
sappelhoff:circle

Conversation

@sappelhoff
Copy link
Copy Markdown
Member

@sappelhoff sappelhoff commented Jan 17, 2020 •
edited
Loading

closes #81

Issues

  • the website is not rendered correctly. Due to missing dependencies in the Gemfile?
  • the direct link to artifacts (GitHub action) may only work upon merging this, see Note in this README
  • speed up the build by caching everything that needs to be installed (done in sappelhoff@adef5ff)

current artifacts: https://circleci.com/gh/bids-standard/bids-website/6#artifacts/containers/0

see for example the index.html: https://6-38337026-gh.circle-artifacts.com/0/home/circleci/project/_site/index.html

@sappelhoff sappelhoff changed the title try circle build MAINT: Let CircleCI render PRs to improve PR review Jan 17, 2020
@robertoostenveld
Copy link
Copy Markdown
Collaborator

robertoostenveld commented Jan 17, 2020

@poldrack this relates to what we were shortly discussion yesterday regarding building the website for pull requests. Just that you are aware that @sappelhoff already initiated action on this.

effigies
effigies reviewed Jan 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@effigies effigies left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think the main issue is with finding the CSS:

bids-website/_layouts/default.html

Line 68 in fa20bfe

<link rel="stylesheet" href="{{ '/assets/css/style.css' | relative_url }}">

I think we need to drop the initial /. It may cause some issues if we have HTML files in deeper directories, but we can worry about that later.

Edit: Actually, this is probably an issue for all cases where "/assets appears in the template.

.github/workflows/main.yml Outdated
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
artifact-path: 0/html/index.html
circleci-jobs: build
Copy link
Copy Markdown
Contributor

@effigies effigies Jan 21, 2020

Choose a reason for hiding this comment

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

This doesn't seem to be working. Do we just want to add:

.circleci/artifact_path:

0/html/index.html

@effigies effigies mentioned this pull request Jan 21, 2020
Closed
@sappelhoff
Copy link
Copy Markdown
Member Author

sappelhoff commented Jan 21, 2020

@effigies this seems to work now. Thanks to your changes (with minor correction)

@effigies effigies merged commit 065b3a9 into bids-standard:gh-pages Jan 21, 2020
@sappelhoff sappelhoff deleted the circle branch January 21, 2020 16:43
sappelhoff added a commit to sappelhoff/bids-website that referenced this pull request Apr 1, 2020
@sappelhoff
cache gemfiles, related to bids-standard#85
adef5ff
sappelhoff added a commit to sappelhoff/bids-website that referenced this pull request Apr 3, 2020
@sappelhoff
cache gemfiles, related to bids-standard#85
08886e2
sappelhoff added a commit that referenced this pull request Apr 3, 2020
@sappelhoff
cache gemfiles, related to #85
503be10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@effigies effigies effigies left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MAINT: Add website builds for easier PR review

3 participants

@sappelhoff @robertoostenveld @effigies