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

fix(initbbs): adjust board order to let bid=1 group exist #115

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

holishing
Copy link
Contributor

@holishing holishing commented May 26, 2022

The group whose bid is 1 should be root of (C)lass 分組討論區.
Besides, the creation order of these boards determine their bid number

This patch will allow contributors to easier organize boards and groups when creating their new site
and also make pttweb able to find the entry by the route (/cls/1) for their new PttBBS site

wens added 5 commits July 28, 2021 20:26
It makes sense to check the CPU load in bbsrf, the lightweight shim for
SSH connections, before executing the heavy mbbsd, which might end up
exiting right away due to high CPU load.

This alleviates CPU load during high bursts of connections.
When GeoIP2 support was introduced, the Makefile was mistakenly written
to enable one or the other, when in fact the code supports enabling both
with GeoIP2 as the preferred database.

Fix the Makefile so that both solutions are enabled.
The user agreement requires an explicit "yes" (agree) from the user.
However the prompt is misleading in that 'Y" is capitalized, implying
that it is the default. This leads to users just pressing enter, and
returning to the menu.

Change 'Y" to lower case to signal that an explicit 'y' is required.
Some global variables were declared locally without an extern qualifier.
This leads to link errors with newer toolchains.

Instead, drop the declarations and include "var.h" which provides all
the proper declarations.
@holishing holishing changed the title fix(initbbs): adjust bid order to let bid=1 group exist fix(initbbs): adjust board order to let bid=1 group exist May 26, 2022
group which bid is 1 should be root of (C)lass
Besides, the create order of these boards will decide there bid number
and prevent too much group/board name start with number
Copy link
Contributor

@IepIweidieng IepIweidieng left a comment

Choose a reason for hiding this comment

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

I produced several reviews about the arrangement of the commits.

b.gid = 2;
newboard(fp, &b);

strcpy(b.brdname, "0_Group");
Copy link
Contributor

Choose a reason for hiding this comment

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

The board creation order is changed again.

Is there a way to rearrange the commits so that there is only one commit which rearranges the board creation order?

@@ -119,90 +133,76 @@ static void initBoards() {
b.gid = 3;
newboard(fp, &b);

strcpy(b.brdname, "1...........");
Copy link
Contributor

Choose a reason for hiding this comment

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

The boardheader_t::brdname is changed, so this commit consists of two kinds of changes.

Although it seems that boardheader_t::brdname is not displayed but for sorting only, so the changes in this commit might not affect the apparent behavior (apparent to whom?).

This is different from the previous commit… or isn't it?

@chhsiao1981
Copy link
Contributor

I thought initbbs is more like the default init settings for pttbbs (and as indication of how current pttbbs board structure is organized).

I think we can have some other customized initbbs, but maybe it's better that we still have this default initbbs, and have the other customized initbbs with some other naming (perhaps _postfix, like: initbbs_holishing)

@IepIweidieng
Copy link
Contributor

As an non-crew of PTT, I am not sure whether the current default is still consistent with the PTT site. (The default seems to be untouched for 19+ years)

A question is whether BID 1 should be assigned to the board SYSOP (as the current default) or to the root catalog group (as proposed). (Is catalog groups exist 25 years ago? Why BID 0 did not be used?)

As for the suffix idea, I think that it should be the other way around, i.e., not initbbs.c/initbbs_holising.c but initbbs_ptt.c/initbbs.c.
It might be understandable for the maintainers of future PttBBS-based sites to use a more reasonable default.

Besides, a way to avoid code duplication might be necessarily, e.g., the suffixed version just invokes the non-suffixed one with specified arguments.

@chhsiao1981
Copy link
Contributor

hi @IepIweidieng and @holishing

I am not a crew of PTT either, nor has the right to merge / reject the PR~

Copy link
Contributor

@hungte hungte left a comment

Choose a reason for hiding this comment

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

I thought initbbs is more like the default init settings for pttbbs (and as indication of how current pttbbs board structure is organized).

The PTT1 is a very special site, so the initbbs and most of the default config are actually designed to work with a "normal" BBS, or something like PTT2.

I think we can have some other customized initbbs, but maybe it's better that we still have this default initbbs, and have the other customized initbbs with some other naming (perhaps _postfix, like: initbbs_holishing)

Definitely not. I think that can be configured in a different way, for example by moving the boards and permissions to an external file (or in pttbbs.conf). Let's not messing the source repo.

A question is whether BID 1 should be assigned to the board SYSOP (as the current default)

Historically it should, but I also can't find any code relying on BID=1. On the contrary, web /cls/1 is hard-coded.

Why BID 0 did not be used?)

0 is used to indicate "error" or "no such board" in get_bnum so we can't use it.

or to the root catalog group (as proposed). (Is catalog groups exist 25 years ago?

I think we can solve the problem in either way: (1) allow the web to support changing root class to a different bid, or (2) revise the init board generation so it's taking an extra input file instead of hard-coded in source.

No matter what, this commit must be changed. We should not make the code more messy.

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.

5 participants