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

Allow multiple repo #125

Merged
merged 11 commits into from
Jan 12, 2024
Merged

Allow multiple repo #125

merged 11 commits into from
Jan 12, 2024

Conversation

alasdairwilson
Copy link
Collaborator

@alasdairwilson alasdairwilson commented Jan 3, 2024

This moves the repo configuration from an env variable to the config yaml file.

repos are configured by:

  • [repo]:
    • the repo name, use a unique identifier for each repo you wish to fetch.
    • path:
      : Unique identifier for the repo, this is used for subdpath in the /material path on the deployed site.
    • url:
      : The URL of the material repo, e.g. https://github.com/UNIVERSE-HPC/course-material.
    • exclude:
      : You can exclude certain sections and courses from the material repo, this is useful if you want to include a subset of the material in your deployment.
      • theme:
        : A list of themes to exclude from the material repo, this should match the "id" field in the theme markdown page.
      • course:
        : A list of courses to exclude from the material repo, this should match the "id" field in the course markdown page.
      • section:
        : A list of sections to exclude from the material repo, this should match the "id" field in the section markdown page.

Support for multiple repos:

  • by adding multiple entries to the config file, multiple repos can be used as sources
  • all repos are combined as cards on /material
  • subsequent urls have the url config var prepended such that it would be /materia/urlA/[repoA material] etc.

Support for excluding parts of the repo:

  • All levels of material can be excluded, by putting an exclude in the config of a repo at the theme, course or section level, those themes, courses and sections and all child parts are excluded.

TODO:

This isn't complete:

  • urls returned from search are busted, this needs reworked
  • pullmaterial, the nightly refresh needs fixed: it can be ported to use the new initrepos function/package.json script (one of)
  • there are no tests: follow up, no idea how we can test this.
    • I suspect we should have a test repo and check it pulls correctly, and check some excludes. I think in general there should be a test.yaml in the config folder for various reasons. This is probably extremely low priority
  • The logic for filtering the nodes in the treeview is overly complex, there are redundant sections (this should be spun off:
    • The issue here is that the component call chain is very very very large for these graphs and that some of the graph by necessity is populated by the material object (which we have successfully excluded) but other times parts are populated by requirements defined in the material files themselves. If these have been excluded then this will cause issues so at any point we are traversing nodes or making edges/links we need to check the exclude list to make sure we are not traversing a forbidden part of the tree. This has been done absolutely everywhere but there are some parts of the tree which are only reached with the safe method (filtered material object).

@alasdairwilson alasdairwilson marked this pull request as ready for review January 5, 2024 16:24
Copy link
Contributor

@martinjrobins martinjrobins left a comment

Choose a reason for hiding this comment

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

This looks great, happy to merge as is, thanks @alasdairwilson!

@martinjrobins martinjrobins merged commit e7cc833 into main Jan 12, 2024
2 checks passed
@martinjrobins martinjrobins deleted the allow-multiple-repo branch January 12, 2024 16:14
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.

2 participants