-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add research menu and relevant docs #157
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Did you just copy pasted the files? It might be better to use git submodules or do some scripting. |
c630b1b
to
6dcbb68
Compare
@fryorcraken I've experimented with using submodules and scripts for Docusaurus, but I've found these methods to be somewhat complicated to maintain. Do we have similar cases like this? If not, let me take the submodule approach but I'm uncertain if everyone is familiar with using submodules. FYI, Below JS script works to fetch the files but "https://api.github.com/repos/" has a rate limit. const https = require('https');
const fs = require('fs');
const path = require('path');
async function fetchFromGitHub(url, callback) {
https.get(url, { headers: { 'User-Agent': 'Node.js' } }, (res) => {
let data = '';
res.on('data', (chunk) => {
data += chunk;
});
res.on('end', () => {
callback(null, JSON.parse(data));
});
}).on('error', (err) => {
callback(err, null);
});
}
async function fetchDirectoryContents(dirUrl, basePath, prefixToRemove) {
fetchFromGitHub(dirUrl, async (err, files) => {
if (err) {
console.error('Error fetching files:', err.message);
return;
}
for (const file of files) {
const relativePath = file.path.replace(new RegExp(`^${prefixToRemove}`), '');
const filePath = path.join(basePath, relativePath);
if (file.type === 'file') {
await downloadAndSaveFile(file.download_url, filePath);
} else if (file.type === 'dir') {
await fetchDirectoryContents(file.url, basePath, prefixToRemove); // Recursive call for subdirectories
}
}
});
}
async function downloadAndSaveFile(url, filePath) {
const fullFilePath = path.join(__dirname, filePath);
https.get(url, (res) => {
const directory = path.dirname(fullFilePath);
// Ensure the directory exists
fs.mkdirSync(directory, { recursive: true });
const fileStream = fs.createWriteStream(fullFilePath);
res.pipe(fileStream);
fileStream.on('finish', () => {
fileStream.close();
console.log('Downloaded and saved:', filePath);
});
}).on('error', (err) => {
console.error('Error downloading file:', err.message);
});
}
const repositories = [
{
baseUrl: 'https://api.github.com/repos/waku-org/nwaku/contents/docs/benchmarks',
baseSavePath: '/docs/research/benchmarks/',
prefixToRemove: 'docs/benchmarks/'
},
{
baseUrl: 'https://api.github.com/repos/waku-org/research/contents/docs',
baseSavePath: '/docs/research/research-and-studies/',
prefixToRemove: 'docs/'
}
];
repositories.forEach(repo => {
fetchDirectoryContents(repo.baseUrl, repo.baseSavePath, repo.prefixToRemove);
}); |
I can see 3 ways of doing things:
|
The script above is intended for use as a build script. (Option 2) I think that incorporating submodules might complicate maintenance. If acceptable, I propose adding the script above to the repository. |
55eb672
to
7007537
Compare
Added the script : https://github.com/waku-org/docs.waku.org/pull/157/files#diff-aac0d1b75493663edefcca04603a6378d9199e3a8902a07a225c398e3b7c0a52 and updated package.json and README |
@fryorcraken Can you please confim if the current "build script" way is okay? You can run |
I'll try locally later. @LordGhostX please also review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this approach appealing and it works very fine. My only comment is: can we make all page headings title capitalized?
"incentivization" -> "Incentivization"
"nwaku Benchmarks" -> "Nwaku Benchmarks"
Also, the docs site uses British English, so "Incentivization" should be written as "Incentivisation", running yarn check:spell
should throw errors because of this
Should be added to the incentivization.md file in the research repo cc @jm-clius
The issue is that this needs to be fixed upstream in the research repo. @jm-clius how do you feel about adding a spellcheck in the research repo? it helps with typo but does add an extra step. We preferably would want to do that directly in the research repo so it does not block the PR in the docs repo. Same question for @Ivansete-status re nwaku repo. Current cspell error output:
If cspell can be added to nwaku and research, then the matching cspell config files should be pulled in too: https://cspell.org/docs/getting-started/#command-lint--spell-checking
So that once spelling is sorted in nwaku/research repo, it will work here. |
@jm-clius if you are able to do the same as waku-org/nwaku#2383 then we could get it shipped! |
Done in waku-org/research#83 |
@jinhojang6 @LordGhostX . Hanno and Ivan have added cspell checks. Can you now review and see if we can merge? |
4507b6c
to
0a632ee
Compare
0a632ee
to
4507b6c
Compare
4507b6c
to
2c00034
Compare
@fryorcraken we're good to merge now 👍 @jinhojang6 I added |
Handle issue #155