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

[rfc] refactor bookcfg #17

Merged
merged 6 commits into from
May 29, 2021
Merged

[rfc] refactor bookcfg #17

merged 6 commits into from
May 29, 2021

Conversation

pietroppeter
Copy link
Owner

  • it really did not make any sense to have it in book, should be in src.
  • also it is not a config, it is the set of defaults of nimibook (so renamed accordingly)
  • and I think it makes sense to create a nimibook / prelude that initializes a nimib document with nimibook defaults. it is something that you would include at beginning of document (similar to include karax / prelude but it does a bit more since it also initializes the nimib document)

since I made a few changes recently without discussing much, I keep this PR open for comments of contributors. @Clonkk, @HugoGranstrom, @zetashift, let me know what you think, feel free also to rediscuss/criticize previous changes about bookcfg.

@Clonkk
Copy link
Contributor

Clonkk commented May 28, 2021

IMO bookcfg being in books is bad because book does not (currently) gets deployed by doing nimble install.

Therefore, include ../../bookcfg will not work for someone who does nimble install

@pietroppeter
Copy link
Owner Author

IMO bookcfg being in books is bad because book does not (currently) gets deployed by doing nimble install.

Therefore, include ../../bookcfg will not work for someone who does nimble install

agree, this is covered by this PR (bookcfg, which is nimibook /defaults in this PR is imported not included)

src/nimibook/pubs.nim Outdated Show resolved Hide resolved
@zetashift
Copy link

Just before I went to sleep I thought of a similar situation, I'm glad you were able to implement it a lot faster than me haha.
Anyway I like this current approach, only thing I would like to see added in either the README or in defaults.nim/prelude.nim is how the setup works. E.g. in prelude.nim:

import nimib, nimibook / defaults
## Initializes a nimib document with nimibooks defaults and other stuff about what prelude.nim might do
nbInit
nbDoc.useNimibook

And in defaults.nim document what useNimibook exactly does.

Of course if this is just a temporary thing then I think things are just fine as is.

@Clonkk
Copy link
Contributor

Clonkk commented May 28, 2021

Looks good to me.

@HugoGranstrom
Copy link
Collaborator

Glad you managed the circular deps. This looks good to me as well :)

@pietroppeter
Copy link
Owner Author

thanks all! I will incorporate the suggestions of @zetashift and will merge later this evening. I was also planning to make a submission for this project in "This Month in Nim" for the month of May (to support the initiative and give visibility) and my idea was to mention you all as contributors in the description (whatever the state of commits, discussing, using, reporting issues is definitely contributions). I will update you when I have prepared a draft of the submission. the idea is also this weekend for me is to work on issues #10, #9 so that the repo is in a presentable state when people give a look at it.

@HugoGranstrom
Copy link
Collaborator

@pietroppeter sounds like a great idea! 😄 The more people who are aware of the greatness of Nimib and NimiBook the better! Would be an honor to be mentioned as a contributor 🤯

Didn't even notice that hidden comment 🙃 Feels like we have come a long way in fixing the most essential of those features at least. I mean except from choosing the default theme + next/prev buttons + search there isn't that much in there that is specific to NimiBook (ie should be in Nimib). Except adding in some Nimib links of course :)

Next month maybe we could submit SciNim/getting-started to prolong the exposure of NimiBook even more 😮

@Clonkk
Copy link
Contributor

Clonkk commented May 28, 2021

I was also planning to make a submission for this project in "This Month in Nim"

That is a great idea

I mean except from choosing the default theme + next/prev buttons + search there isn't that much in there that is specific to NimiBook

#12 Needs to be solved. First thing people will try is creating a book and currently there is no "proper" solution to do that.

Next month maybe we could submit SciNim/getting-started to prolong the exposure of NimiBook even more open_mouth

For now getting-started is empty. We should probably aim to write some content before advertising it 😆

@HugoGranstrom
Copy link
Collaborator

HugoGranstrom commented May 28, 2021

#12 Needs to be solved. First thing people will try is creating a book and currently there is no "proper" solution to do that.

For now, the template functionality would suffice? Or just cloning the repo if one would want to do it locally without involving a Github repo. But a nice executable would be optimal of course 🤔 And how are we going to migrate to NimiBook from what we currently have? An executable wouldn't have made it easier to transfer though. Yesterday when working on NimiBoost I removed all code from the the repo and copy-pasted in a newly generated Extension-template so it does work which in a pinch at least 😝

For now getting-started is empty. We should probably aim to write some content before advertising it 😆

We have entire month to write content, that's lots of time... 🤣

@pietroppeter
Copy link
Owner Author

added a nimble srcpretty task that runs nimpretty --maxLineLen:160 over all nim files in src directory.

added in either the README or in defaults.nim/prelude.nim is how the setup works.
And in defaults.nim document what useNimibook exactly does.

@zetashift I will add those when working on #10

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