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

CCSMB-4: Lua files #9

Closed
wants to merge 6 commits into from
Closed

Conversation

tomodachi94
Copy link
Member

Imported from oeed/CraftOS-Standards.

@tomodachi94 tomodachi94 added the classification: proposal Introduction of a new proposal. label Nov 10, 2022
Standards/CCSMB-4.md Outdated Show resolved Hide resolved
Standards/CCSMB-4.md Outdated Show resolved Hide resolved
Standards/CCSMB-4.md Outdated Show resolved Hide resolved
Standards/CCSMB-4.md Outdated Show resolved Hide resolved
@tomodachi94 tomodachi94 requested review from Ocawesome101, Erb3, fatboychummy and znepb and removed request for fatboychummy and znepb November 10, 2022 20:18
@tomodachi94
Copy link
Member Author

My bad, the button is being weird.

@tomodachi94 tomodachi94 requested review from znepb and removed request for fatboychummy November 10, 2022 20:18
Copy link

@znepb znepb left a comment

Choose a reason for hiding this comment

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

Make definition that this is exclusively lua text files, not bytecode

@tomodachi94
Copy link
Member Author

tomodachi94 commented Nov 10, 2022

Make definition that this is exclusively lua text files, not bytecode

Fixed in 119213f.

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.

I personally don't think that just a filename extension should be a whole standard. Considering API design (using require over os.loadAPI) and coding practices (globals vs. locals) are hot topics of debate in the community, we should use this document to standardize some of those interfaces - the main one being to promote the use of require and modules that don't have side-effects. To avoid conflict, we should avoid personal style preference (tabs vs. spaces, 2 vs. 4 indentation width), but stuff that actually affects the computer, and readability of code, should be standardized.

@MasonGulu
Copy link

I personally don't think that just a filename extension should be a whole standard. Considering API design (using require over os.loadAPI) and coding practices (globals vs. locals) are hot topics of debate in the community, we should use this document to standardize some of those interfaces - the main one being to promote the use of require and modules that don't have side-effects. To avoid conflict, we should avoid personal style preference (tabs vs. spaces, 2 vs. 4 indentation width), but stuff that actually affects the computer, and readability of code, should be standardized.

I agree with this, it'd allow the standard to have more purpose than simply declaring a file extension. The original oeed standards did not do anything beyond specifying the filename.

Maybe create a list of bad practices these files should avoid, or better yet good practices the files should use.

  • Use require when importing libraries
  • Avoid necessary use of globals

My language abilities are lacking, and these are the only two things I can think of off the top of my head currently.

@EmmaKnijn
Copy link
Member

Alright, a few things to note, how should we format the .md file itself (including the header), personally fixed linebreaks aren't preferred. I tried to push a first iteration of the code but I don't have permission, so I'll look into that.

@EmmaKnijn
Copy link
Member

EmmaKnijn commented Nov 12, 2022

Was able to throw the changes into the ccsmb-4-additions branch, which is a bit messy bit gets the job done (I think haha)

Copy link
Member

@EmmaKnijn EmmaKnijn left a comment

Choose a reason for hiding this comment

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

Alright, I think I managed to do it the proper way

EDIT: Nevermind, I think we just gotta wait on @tomodachi94 for them to turn on the "allow edits from maintainers" option

@tomodachi94
Copy link
Member Author

Alright, I think I managed to do it the proper way

EDIT: Nevermind, I think we just gotta wait on @tomodachi94 for them to turn on the "allow edits from maintainers" option

Fixed, try pushing now.

@EmmaKnijn
Copy link
Member

Thanks!

@MCJack123
Copy link
Contributor

I'm working on a pretty big formal standard to replace this one - please bear with me until I can finish it up.

@tomodachi94
Copy link
Member Author

Closing in favor of #16.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants