Skip to content

Change SCons output structure #963

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

Conversation

PeanutbutterWarrior
Copy link
Contributor

@PeanutbutterWarrior PeanutbutterWarrior commented Dec 1, 2021

The PR for #954
Currently:

  • Changes the output structure to the new one for all builds
  • Adds aliases for languages to just use their name e.g. scons rust
  • Searches chapters recursively for source files
  • Adds aliases for chapters

@PeanutbutterWarrior PeanutbutterWarrior marked this pull request as ready for review December 1, 2021 23:35
@ntindle
Copy link
Member

ntindle commented Dec 2, 2021

This seems to work to me. Not one of the people who originally weighed in though

Copy link
Member

@leios leios left a comment

Choose a reason for hiding this comment

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

I think this looks great! I would have loved to see this with "edge cases" like the code for the convolution chapter to make sure it works, but I think it is fine as is.

I'll wait a second to merge it in case there are more informed opinions

@PeanutbutterWarrior
Copy link
Contributor Author

I've tested nested chapters with dummy files and it works as far as I can tell. I've got no idea what will happen if there's a code directory inside another, so we should avoid that (although I don't know how that would happen). This also doesn't replicate the structure inside the individual language directories, so if that matters it will need an SConscript file.

@Amaras Amaras merged commit 946d4f1 into algorithm-archivists:main Dec 5, 2021
Amaras added a commit to Amaras/algorithm-archive that referenced this pull request Dec 5, 2021
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.

4 participants