Skip to content

Fix incorrect date format for Czech and Slovak Republic#2

Merged
PerditionC merged 1 commit intoFDOS:masterfrom
jmalak:patch-1
Dec 30, 2021
Merged

Fix incorrect date format for Czech and Slovak Republic#2
PerditionC merged 1 commit intoFDOS:masterfrom
jmalak:patch-1

Conversation

@jmalak
Copy link
Copy Markdown
Contributor

@jmalak jmalak commented Oct 3, 2021

Both Countries use DD.MM.YYYY date format.
Microsoft DOS also use this format.

Both Countries use DD.MM.YYYY date format.
Microsoft DOS also use this format.
@jmalak
Copy link
Copy Markdown
Contributor Author

jmalak commented Oct 3, 2021

It is very confusing that you have two repositories to same thing.
One is Kernel with defaults and COUTRY.SYS for override by configuration.
I thing COUNTRY.SYS file should be part of kernel repository to hold current and default values together to don't hold different values or should be cross-referenced in both source files.

@andrewbird
Copy link
Copy Markdown
Contributor

Ideally country repo would be a submodule of kernel. Unusually it would still be necessary to update two repos: kernel for the default: and country for the other. I suppose we could move the defaults into the country repo, even though it wouldn't actually be used there. Thoughts?

@jmalak
Copy link
Copy Markdown
Contributor Author

jmalak commented Oct 4, 2021

Sorry, I am new to FD that my experiences with FD project organization/rules is minimal.
I don't use DOS regularly only randomly as test platform, that lot of things I forgot.
I think both should use same source conditionally but it is hard if they are divided as now.
Cross-reference to each other on top of source files could be OK.
Anyway what is reason for holding a lot of country info in the kernel, I am not sure how it is in MSDOS but probably only US info are there and rest is in COUNTRY.SYS and must be configured explicitly.

@bttrx
Copy link
Copy Markdown

bttrx commented Dec 30, 2021

@andrewbird @jmalak @PerditionC Please, can we go on with the discussion and come to some decisions?

@andrewbird
Copy link
Copy Markdown
Contributor

Hi Robert,
I'm happy to take the updates here and make the kernel use this repo as a submodule (so removing its own copy of country.asm). Regarding the kernel defaults I'm thinking it might be more maintainable to have them here also but as a separate file that the kernel can include. @PerditionC are you in agreement with that scheme? Regarding @jmalak's thoughts on holding only US info in the kernel, I'm probably not the right person to consider this as being a native English speaker I've not had much cause to fiddle with country settings except perhaps to enable the £ sign, so the consequences of missing country specific info in the kernel are not clear to me.

@andrewbird
Copy link
Copy Markdown
Contributor

@PerditionC I've made a little tweak as per Robert's comment in #1, you should merge @jmalak's #2 (this PR) and my new PR #3 to make this repo the up to date one. Then if you are in agreement I'll start on making this a suitable submodule?

@PerditionC PerditionC merged commit 5e23a5c into FDOS:master Dec 30, 2021
@jmalak jmalak deleted the patch-1 branch December 30, 2021 14:01
@andrewbird
Copy link
Copy Markdown
Contributor

The conversion to submodule is complete and merged into both country and kernel repos. If you want to checkout the result download the kernel build artefact file from here https://github.com/FDOS/kernel/actions/runs/1638169003 you should be able to test both the gcc and watcom compiled kernels and the country.sys that was built alongside them. All future updates for both should go the to https://github.com/FDOS/country repo.

@PerditionC
Copy link
Copy Markdown
Contributor

Thank you

@bttrx
Copy link
Copy Markdown

bttrx commented Dec 30, 2021

Thanks for your efforts.
I tested your watcom-built country.sys artifact with (049,858) on SvarDOS and FreeDOS 1.3 RC5 successfully.

@andrewbird
Copy link
Copy Markdown
Contributor

Thanks for testing.

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