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

create 2-3 new Documents for VFS,VGZ,SEA documents #21

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

walksanatora
Copy link

this is a draft so that i can recieve critiscism on how i wrote it, and get pointers for writing better documents (gonna finish them 1 by 1)
the first question is though, should i have VGZ be a diffrent document despite being VFS but GZipped

@tomodachi94 tomodachi94 added the classification: proposal Introduction of a new proposal. label Jan 9, 2023
@walksanatora
Copy link
Author

so i am looking for input on the VFS document
if it is all good i will then ask
should i make VGZ a seperate document or a addendum
(also i think SEA will be more of a general "concept" mabey)

Copy link
Contributor

@MCJack123 MCJack123 left a comment

Choose a reason for hiding this comment

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

Right now, the document is very vague and doesn't really describe the data beyond "it has keys and values". There should be examples of what it looks like, covering any potential edge cases, as well as code to encode and decode it, including basic file reading/writing. It also needs some grammar improvements to match the professional tone requirements (mainly capitalization and punctuation).

  1. VFS is fine to standardize, though we may want to standardize something more rich (including access/modification times, and possibly other possible metadata). I have a format that will be used in Phoenix (not published yet) which has metadata including permissions, but that may be out of scope for CraftOS-centric formats.
  2. VGZ is unnecessary to standardize - it's just a gzipped VFS file. You may mention that gzip is a recommended compression format, but it's not a separate format itself.
  3. As far as I know, SEA uses custom code to extract inside the file. We shouldn't be standardizing any specific code anywhere - we only provide protocols for coders to use so their different programs are able to communicate with each other. There's also no use case for programmers, as it only functions when executed, and only has a single purpose (extract files). In the end. it becomes a document saying that a file with extension .sea (which isn't even recognized as a program by default) should extract files, which isn't much of a standard.

@walksanatora
Copy link
Author

updated document a bit
requesting recheck

@walksanatora
Copy link
Author

1 possible change that i can make is simplifying my code into two seperate files so you dont have to look at my boilerplate to make sure arguments are correct

@walksanatora
Copy link
Author

that and commenting my program

Copy link
Contributor

@MCJack123 MCJack123 left a comment

Choose a reason for hiding this comment

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

Looking better, though it still has a few typos. Also, the examples are way too long - they should be less than 15 lines long, and show only one thing at a time. You should avoid machine-generated example files - write them out by hand with adequate spacing to make them readable.

Also, don't forget to mention recommending gzip for compression here.


requesting recheck

Heads up that this button exists:
image

Standards/CCSMB-X.md Outdated Show resolved Hide resolved
Standards/CCSMB-X.md Outdated Show resolved Hide resolved
Standards/CCSMB-X.md Outdated Show resolved Hide resolved
Standards/CCSMB-X.md Outdated Show resolved Hide resolved
Standards/CCSMB-X.md Outdated Show resolved Hide resolved
VFS files should be read and written in binary mode as to prevent converting binary charachters to `?`
One drawback of the VFS format is that it does not save any MetaData of files compressed

## Examples included
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend having the examples embedded inline with descriptions of what they do. This makes it easier to read quickly.

Copy link
Author

Choose a reason for hiding this comment

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

but it is a full program, or should i have the Examples section contains example functions for creating/extracting (the gen_disk and unpack_vfs functions) along wit a small text example
and have the full example program and example image in the folder

Copy link
Contributor

Choose a reason for hiding this comment

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

You shouldn't be posting the full program here - put that on your own repo and link to it as an additional example. The examples here should be limited to single functions that demonstrate just one part at a time.

Copy link
Author

Choose a reason for hiding this comment

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

so should i embed the example functions i use to create/unpack the archive with comments
but just those functions
and include a simpler example since having a folder and 1 lua script and a 18kb text file is a bit mutch

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but try to make them as simple and easy to read as possible (e.g. assume the file's correct, no error checking).

Copy link
Author

Choose a reason for hiding this comment

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

thats what they do allready
i do all the error checking outside of the functions anyway

Copy link
Author

Choose a reason for hiding this comment

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

i think it is better now

Standards/CCSMB-X.md Outdated Show resolved Hide resolved
Archive not Archiv

Co-authored-by: Tomo <[email protected]>
@walksanatora walksanatora requested review from tomodachi94 and MCJack123 and removed request for MCJack123 and tomodachi94 January 12, 2023 04:46
@tomodachi94 tomodachi94 added status: reviews needed A proposal needs reviews. status: stale An unmerged pull request has sat for a month with no comments. labels Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
classification: proposal Introduction of a new proposal. status: reviews needed A proposal needs reviews. status: stale An unmerged pull request has sat for a month with no comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants