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

[Repository] Add .github folder with contributing file #3362

Closed
wants to merge 20 commits into from

Conversation

Ash258
Copy link
Contributor

@Ash258 Ash258 commented Apr 23, 2019

Present CONTRIBUTING.md file.

This file is showed before someone open their first PR.

Next steps:

  1. Issue templates
  2. PR Templates
  3. Extend new_issue_msg result url to contains issue template

Help with wording, content of contributing file is welcomed

Ash258 and others added 2 commits May 20, 2019 16:14
Co-Authored-By: Richard Kuhnt <r15ch13+git@gmail.com>
Co-Authored-By: Richard Kuhnt <r15ch13+git@gmail.com>
Co-Authored-By: Richard Kuhnt <r15ch13+git@gmail.com>
@r15ch13
Copy link
Member

r15ch13 commented May 20, 2019

Checked spelling and and rewritten some small parts.

# Contributing

There are two main ways how to contribute to the Scoop codebase.

1. [Extending codebase of scoop itself](#core-codebase)
1. [Writing/updating manifests](#manifest-creation)

If you have a question regarding Scoop overall, feel free to head into community Discord server. (Invitation link inside [README][README])

## Core codebase

You can contribute to core codebase these ways:

1. [Implement new features](https://github.com/lukesampson/scoop/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Aenhancement)
1. [Fix bugs](https://github.com/lukesampson/scoop/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Abug)
1. Identify/report reproducible steps of issues

Scoop's core codebase started moving towards standard/preferred PowerShell code style. Mainly with the adoption of [`Verb-Noun`][approved-verbs] naming.
Starting from April 2019 use `Verb-Noun` naming for functions when manipulating the codebase.

TODO:

- Verb-Noun refactor flow
    - Move code to new `Verb-Noun` function
    - Add Depreaction warning (using [`Show-DeprecatedWarning`][Show-DeprecatedWarning])
    - Call new function in old
    - Replace all references to old function
- functions parameters, CmdletBinding(), naming, ...
    - Why CmdletBinding
        - Advanced function (practically equaled with scripts)
        - Default parameters (Debug, Verbose, ...)
- No aliases
    - Obvious reasons

## Manifest creation

All official buckets are community driven and everyone can add, update and edit them.
All Pull-Requests are warmly welcomed. The next few lines will guide you on how to write manifests, which pass our standards without any problems.

❗ If you haven't read the Wiki please do it now. You could visit the [projects Wiki](https://github.com/lukesampson/scoop/wiki/App-Manifests) or the [exprimental external documentaion](https://scoop.netlify.com/concepts/#app-manifests) ❗

The Wiki gives you a basic insight on how manifests should look like and how everything works together.

A small overview of guidelines which you need to follow when writing manifests. (Explanation to each of them below)

1. Manifests need to be formatted as described in [.editorconfig file][.editorconfig]
    - 4 spaces indentation
    - New line at end of file
    - No trailing whitespace
2. Always include these informative properties:
    - `description`
    - `license`
3. Write readable/extendable code inside script blocks
4. Test your auto-updates before publishing a Pull-Request
5. Always inspect if the vendor of the application provides checksums for artifacts

### Manifest format

When you publish a Pull-Request the AppVeyor pipeline will be executed and check for formatting errors inside your manifests.
Best practice for always passing all checks is to run [checkver][checkver] or [format][formatjson] scripts before posting Pull-Request.

These are PowerShell scripts which should run on all platforms (Linux, MacOS with `pwsh`), so you are not tied with Windows platform.

### Readable code

You can specify `post_install`, `pre_install` and `installer.script` blocks, which could be an array (or a simple string) with PowerShell code.
For these blocks use the syntax as you would normally write PowerShell scripts (Follow PSScriptAnalyzer rules; See: [core codebase](#core-codebase)).

How script blocks should **NOT** look like: <https://github.com/lukesampson/scoop/blob/fa6ccc9471a29bf621c80a507d387a371293de75/bucket/jetbrains-toolbox.json#L32>

### Autoupdates

❗ Always test your auto-updates before posting a Pull-Request ❗

The last step what you should do before posting a Pull-Request is to run the [checkver][checkver] script with the `-Force` (`-f`) parameter.
This will check for the latest version of the program (specified within the `checkver` property) and update the manifest file.

- Update all properties inside `autoupdate` property with actual values (URL, `extract_dir`, ...)
- Calculates or extracts checksums for all artifacts
- Formats the manifest to comfort standard

[README]: ../README.md
[.editorconfig]: ../.editorconfig
[checkver]: ../bin/checkver.ps1
[formatjson]: ../bin/formatjson.ps1
[Show-DeprecatedWarning]: https://github.com/lukesampson/scoop/blob/6141e46d6ae74b3ccf65e02a1c3fc92e1b4d3e7a/lib/core.ps1#L22-L36
[approved-verbs]: https://docs.microsoft.com/en-us/powershell/developer/cmdlet/approved-verbs-for-windows-powershell-commands

@Ash258
Copy link
Contributor Author

Ash258 commented May 20, 2019

@r15ch13 You should be able to push into this branch. Feel free to push directly instead of writing some diffs or anything.

Or create patch and post it here so i can apply.

@r15ch13
Copy link
Member

r15ch13 commented May 20, 2019

Copy and paste will do fine 😄

@Ash258
Copy link
Contributor Author

Ash258 commented May 20, 2019

@r15ch13 In Markdown you do not need to number "steps" inside numbered lists.

1. COSI
1. Alfa
1. BETA

Will become automatically this when rendered:

1. COSI
2. Alfa
3. BETA

A

Co-Authored-By: Richard Kuhnt <r15ch13+git@gmail.com>
@Ash258 Ash258 changed the base branch from master to develop July 27, 2019 21:09
@Ash258
Copy link
Contributor Author

Ash258 commented Jul 27, 2019

Should I mention something like logical manifest properties order?

In all my buckets and when i am training some internal bucket maintainers I use Template with comments and basic logical manifest ergonomics regions

  1. General information part
    • version, description, homepage, license, notes,
  2. Requirements / Depends
    • Depends, suggests
  3. Downloading
    • cookies, url, hash
  4. Extraction
    • innosetup, extract_dir, extract_to
  5. Installing
    • pre, installer, post, uninstall
  6. Links
    • bin, shortcuts, modules, path, env, persists
  7. Updating
    • Checkver, autoupdate

@rasa
Copy link
Member

rasa commented Jul 27, 2019

Should I mention something like logical manifest properties order?

+1 here. Standardizing on a particular order of tags is more important to me than the specific order.

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 22, 2019

Note to self and discussion starters

Remaining items to be added and specified:

1. Logical manifest properties order sorted by execution (#3362 (comment))
1. PR / commits (for collaborators) naming (#3597 (comment))

  1. Rules to write descriptions. (Add 172 descriptions Main#291 (comment))

If anyone have specific request feel free to suggest.

@linsui
Copy link
Contributor

linsui commented Aug 22, 2019

Rules to name an app should be similiar to descriptions.
There are some apps with confusing or lengthy name, e.g. calibre-normal, listen1desktop, listenmoeclient.
I prefer to use app-name instead of appname, especially for those long names, e.g xmousebuttoncontrol, winsshterm, tinymediamanager.
When should we contain version in appname, e.g. pypy3 and python?
And when can we use abbreviation, e.g. pwsh?

@Ash258
Copy link
Contributor Author

Ash258 commented Aug 22, 2019

I will just put it here as note to self https://www.conventionalcommits.org/en/v1.0.0-beta.4/#specification

Co-Authored-By: Richard Kuhnt <r15ch13+git@gmail.com>
Copy link
Contributor

@ddavness ddavness left a comment

Choose a reason for hiding this comment

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

This is basically just a (very) big dive on the document from mostly a grammar perspective (the document contains some errors that need to be fixed).

Most of the suggestions are trivial to apply, some can be changed depending on interpretation/intended meaning/etc...

1. [Extending codebase of scoop itself](#core-codebase)
1. [Writing/updating manifests](#manifest-creation)

If you have a question regarding Scoop overall, feel free to head into community Discord server. (Invitation link inside [README][README])
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you have a question regarding Scoop overall, feel free to head into community Discord server. (Invitation link inside [README][README])
If you have a question regarding Scoop overall, feel free to head into the community Discord server. (Invitation link inside [README][README])


## Core codebase

You can contribute to core codebase these ways:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can contribute to core codebase these ways:
You can contribute to the core codebase these ways:

1. [Fix bugs](https://github.com/lukesampson/scoop/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Abug)
1. Identify and report reproducible steps of issues

Scoop's core codebase started moving towards standard/preferred PowerShell code style.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Scoop's core codebase started moving towards standard/preferred PowerShell code style.
Scoop's core codebase started moving towards the standard/preferred PowerShell code style,

@@ -0,0 +1,180 @@
# Contributing

There are two main ways how to contribute to the Scoop codebase.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
There are two main ways how to contribute to the Scoop codebase.
There are two main ways on how to contribute to the Scoop codebase:

1. Identify and report reproducible steps of issues

Scoop's core codebase started moving towards standard/preferred PowerShell code style.
Mainly with the adoption of [`Verb-Noun`][approved-verbs] naming.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Mainly with the adoption of [`Verb-Noun`][approved-verbs] naming.
mainly with the adoption of [`Verb-Noun`][approved-verbs] naming.

We're doing this because as you probably know, GitHub onelines these lines.


<!-- TODO some preface -->

- Do not mention application name
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Do not mention application name
- Do not mention the application name

<!-- TODO some preface -->

- Do not mention application name
- Application name is needed only in case when manifest name is different (in case of more popular acronym for command line utilites for example) from application name
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Application name is needed only in case when manifest name is different (in case of more popular acronym for command line utilites for example) from application name
- The application's name is needed only in case when the manifest's name is different (in case of more popular initials or acronym for some command line utilities, for example, `TODO-insert-some-relevant-manifest-here`).


#### Properties order

Manifest consists of 7 main groups (regions) of properties. These properties (and it's sub-properties) should be logically ordered from top to bottom as they are evaluated in installation process.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Manifest consists of 7 main groups (regions) of properties. These properties (and it's sub-properties) should be logically ordered from top to bottom as they are evaluated in installation process.
A manifest consists of 7 main groups (regions) of properties. These properties (and their sub-properties) should be logically ordered from top to bottom as they are evaluated in installation process.


❗ Always test auto-updates before posting a Pull-Request ❗

The last step what should be done before posting a Pull-Request is to run the [checkver][checkver] script with the `-Force` (`-f`) parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The last step what should be done before posting a Pull-Request is to run the [checkver][checkver] script with the `-Force` (`-f`) parameter.
The last step that should be done before posting a Pull-Request is to run the [checkver][checkver] script with the `-Force` (`-f`) parameter.


- Update all properties inside `autoupdate` property with actual values (URL, `extract_dir`, ...)
- Calculates or extracts checksums for all artifacts
- Formats the manifest to comfort standard
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Formats the manifest to comfort standard
- Formats the manifest to conform the standard

@Ash258 Ash258 closed this May 9, 2020
@Ash258 Ash258 deleted the github branch June 21, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants