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

Make Vinum Contributor-Friendly #31

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions CONTRIBUTING.MD
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Contribution
All contribution to Vinum is highly welcomed. However, to ensure clarity and that nobody's time is wasted here, we enforce a contribution guideline.

## Guidelines

### Bug Reporting
Vinum is a new and immature library, as such, bugs are expected to arise. Read this to learn more about how to report them effectively to the Vinum Team.

Before opening an issue, make sure all of the following is met:
* **You are the first one to report it**. To ensure that, you should check both *clsoed* and *opened* issues.
* **If a new variant of a bug was introduced after the original one was fixed**, it's best to open a new issue.
* **When opening a new issue**, the title should be ***simple and sensible***, with a summary of as much relevant information as possible in addition to simple reproduction steps with a .rbxl file.
* **When a fix has been merged** (and verified to work) onto the `master` branch of Vinum, the issue will be closed.
* **Please be patient**, as the maintainer team and contributors are volunteers. Bumping issues with `new updates` doesn't help with this.

### Feature Requests
While we work to make Vinum packed with features that satisfy universal cases, some slip away. Read this to learn more about how to suggest new features to the Vinum Team.

Before opening an issue, make sure all of the following is met:
* **You are the first one to suggest it**. To ensure that, you should check both *closed* and *opened* issues. It's possible your feature was rejected.
* **If you do think the issue shouldn't be rejected and have a strong reason behind that**, you should always add your thoughts to the *existing* issue.
* **When opening an issue**, the title should be ***simple and sensible***, with an issue body that explains why a feature should be added, and who it would help, when it would and why.
* As long as it's not the main focus, **API design suggestions are allowed**.
* Almost all feature requests do have a valid concern, but **it's certianly possible that it needs work to design an API that matches Vinum's current public API**.
* **When an implementation has been merged** into `master`, the issue will be closed.
* **Your feature should adopt a declarative mindset** so that users shouldn't depend on *imparative* code that depends on execution order.
* **Guide the users of the feature towards optimal and maintainable code**. This also means that mistakes by the developer are easy to spot. In addition, over engineering isn't welcomed.

### Code Contribution
Ever wanted to implement a feature yourself? Well, read this to know how to effectively do so without wasting your precious time.


When working on a pull request, make sure the following is understood:
* **Your pull request implements an approved feature request**, this means that you shouldn't introduce new API design and features in pull requests. This extends to implementing feature requests that still didn't enter the `approved` phase.
* Additionally, extends to pull requests that fix bugs.
* **You should always create a draft rather than a completed pull request**. This means we can see what you're working on and guide you as you work, rather than lumping all feedback at the end of the process.
* **Always make it clear what you are working on**, this means adding to-do lists if the feature being developed is fairly big.
* **Ensure pull requests are focused on a single feature**.
* **Reviews are bound to happen, but these aren't personal**, though if you have a conflicting opinion with the reviewer, feel free to voice it.
* **Some pull requests might be closed if deemed conflicting with Vinum**. This *shouldn't* happen a lot, but when it does, we will make it clear why we decided to do so.


## Repo Management

### Issue Phases
1. When an issue is created, its phase is `discussion` right away. This means that the issue is still being evaluated.
2. After the `discussion` phase, the issue is either rejected and enter the `rejected` phase, or accepted and enter the `designing` phase where a solution will be designed.
3. When the solution is finally designed, the issue will enter the `approved` phase, where it waits for the said solution to merge into the `master` branch so that it can be closed.

### Commit Messages
Vinum adheres to [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0-beta.4/#specification) spec alongside a few different types and scopes.

#### Types
* fix - A code change that fixes a bug.
* feat - A code change that implements a feature.
* chore - Changes to our tooling and external dependency updates, and repo-management changes.
* docs - Documentation only changes
* perf - A code change that improves performance.
* refactor - A code change that doesn't fix a bug or adds a new feature.
* tests - Adds missing tests or correcting existing ones.
* bench - Adds missing benchmarks or correcting existing ones.

#### Scopes
* API - Changes in relation to Vinum's API.
* internal - Changes in relation to Vinum's internal systems such as the dependency system.
* meta - Changes in relation to any non-code component of the repo.
124 changes: 124 additions & 0 deletions style-guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
# Vinum Style Code
This style guide aims to unify as much Luau code in Vinum as possible under the same style and conventions.

The rules and guidelines set out here are derived from Roblox's style guide, which can be found here: https://roblox.github.io/lua-style-guide, additionally, there are certian blocks that were inspired/taken from Fusion's style guide which can be found here: https://github.com/dphfox/Fusion/blob/main/style-guide.md.

## Guiding Principles
* The purpose of a style guide is to avoid arguments
* There's no one right answer to how to format code, but consistency is important, so we agree to accept this one, somewhat arbitrary standard so we can spend more time writing code and less time arguing about formatting details in the review.
* Optimize code for reading, not writing.
*You will write your code once. Many people will need to read it, from the reviewers, to any one else that touches the code, to you when you come back to it in six months.
* All else being equal, consider what the diffs might look like. It's much easier to read a diff that doesn't involve moving things between lines. Clean diffs make it easier to get your code reviewed.
* Avoid magic, such as surprising or dangerous Lua features:
* Magical code is really nice to use, until something goes wrong. Then no one knows why it broke or how to fix it.
* Metatables are a good example of a powerful feature that should be used with care.
* Be consistent with idiomatic Luau when appropriate.

## Folder Structure
Each file is under a specific folder in Vinum's source codebase depending on factors like their behavior.

## File Structure
Files should consist of these things (if present) in order:
1. A comment like `!strict` and other similar command like comments
2. An optional block comment specificing why this file exists.
3. Services used by the file, using `GetService`
4. Module Imports, using `require`
5. Imported Module-level constants
6. Script-Level Constants
7. Script-Level variables and functions
8. The entity the module returns
9. a return statement

## Requires
* `require` calls should be at the top of the file, making dependencies static.
* If there's an issue with two modules requiring each other cyclically, the structure of that code needs to be reconsidered.
* When requiring a module inside Fusion's source code, use relative paths. To keep these paths clean, use a variable called Package to store where the root of the library is.
* This makes it clear where to find the source of the module within the source code.
```lua
local Package = script.Parent.Parent
local Foo = require(Package.Utils.Foo)
local Bar = require(Package.Reactive.Bar)
```
* Elsewhere, prefer absolute paths when requiring modules:
```lua
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Foo = require(ReplicatedStorage.Foo)
```

## Metatables
Metatables are an incredibly powerful Lua feature that can be used to overload operators, implement prototypical inheritance, and tinker with limited object lifecycle.

However, due to the nature of Metatables, the code that makes use of them is usually regarded as magic code, unless they used in cases like:
* OOP classes
* Guarding against typos - though Vinum doesnt make use of this, as such, this use case isnt documented here.

### OOP
Vinum has a very specific design for writing classes. Which is separating constructors from the class tables.

To create a `Math` class, define a `class` table, and a meta table with an `__index` metamethod that points at class:
```lua
local class = {}
local meta = {__index = class}
```

Now, lets create few methods for our class:
```lua
local function class:add(a, b)
return a + b
end
```

Now, lets create our constructor function. To keep everything simple, the constructor's name should be `Math`:
```lua
local function Math()
local self = setmetatable({
-- some members that can be added directly
}, meta)

-- add some members that usually depend on another member
self.hi = self.x + 2

-- return the object!
return self
end
```

While this is everything, Vinum generally enforces a rule where all classes should have manually written type definitons, as such, you need to do the following to your constructor:
```lua
local function Math()
local self = setmetatable({
-- some members that can be added directly
}, meta) :: any

-- add some members that usually depend on another member
self.hi = self.x + 2

-- return the object!
return self :: ClassTypes.Math
end
```


Further additions you can make to your class as needed:
* Add a `meta` string to objects of your class - this is used to differentiate objects of different class types

## General Punctuation
* Don't use semicolons (;). They are generally only useful to separate multiple statements on a single line, but you shouldn't be putting multiple statements on a single line anyway.
* This includes using semicolons in tables; prefer to use commas (,) to delimit values in tables.
* In comments and other documentation, use backticks when referencing code, and indentation when embedding longer blocks of code
```lua
-- `foo` will be set to `5 + os.clock()`
local foo = 5 + os.clock()

--[[
An example implementation of a function using `os.clock()`:

local function getMinutes()
local now = os.clock()

return math.floor(now / 60)
end

The above code returns the integer number of minutes since the epoch.
]]
```