From 4763974700388e56fc99e8628b6897dea46a3645 Mon Sep 17 00:00:00 2001 From: Safwan Samsudeen Date: Sun, 19 Mar 2023 13:33:34 +0530 Subject: [PATCH 1/5] correction of language style and wrong links --- .gitignore | 1 + building/configlet/lint.md | 28 +++++++++---------- .../github/contributors-pull-request-guide.md | 15 ++++------ building/github/gha-best-practices.md | 6 ++-- .../github/maintainers-pull-request-guide.md | 2 +- building/tooling/analyzers/docker.md | 2 +- .../tooling/analyzers/feedback-guidelines.md | 4 +-- 7 files changed, 28 insertions(+), 30 deletions(-) diff --git a/.gitignore b/.gitignore index c2658d7d..2752eb92 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ node_modules/ +.DS_Store diff --git a/building/configlet/lint.md b/building/configlet/lint.md index 8176f7ac..89557b15 100644 --- a/building/configlet/lint.md +++ b/building/configlet/lint.md @@ -526,24 +526,24 @@ The `config.json` file should have the following checks: ## Glossary -1. Non-blank string: a string that contains at least one non-whitespace character. -2. kebab-case string: a string that contains only characters in the range `[a-z0-9]`, optionally separated by dashes (e.g. "two-fer"). It must match the regular expression: `^[a-z0-9]+(-[a-z0-9]+)*$` -3. Title Case string: a non-blank string that follows the below guidelines (from the Chicago Manual of Style, see https://en.wikipedia.org/wiki/Title_case): - > - Capitalize the first and last words of titles and subtitles. - > - Capitalize "major" words (nouns, pronouns, verbs, adjectives, adverbs, and some conjunctions). - > - Lowercase the conjunctions _and_, _but_, _for_, _or_, and _nor_. - > - Lowercase the articles _the_, _a_, and _an_. - > - Lowercase prepositions, regardless of length, except when they are stressed, are used adverbially or adjectivally, or are used as conjunctions. - > - Lowercase the words _to_ and _as_. - > - Lowercase the second part of Latin species names. -4. Sentence Case string: a non-blank string that follows the below guidelines (see https://en.wikipedia.org/wiki/Letter_case#Sentence_case): - > - Capitalize the first word of the sentence, as well as proper nouns and other words as required by a more specific rule. -5. Valid `"files"` pattern: A non-blank string¹ that specifies a location of a file used in an exercise, relative to the exercise's directory. A pattern may use one of the following placeholders: +1. **Non-blank string**: a string that contains at least one non-whitespace character. +2. **Kebab-case string**: a string that contains only characters in the range `[a-z0-9]`, optionally separated by dashes (e.g. "two-fer"). It must match the regular expression: `^[a-z0-9]+(-[a-z0-9]+)*$` +3. **Title-case string**: a non-blank string that follows the below guidelines (from the [Chicago Manual of Style](https://en.wikipedia.org/wiki/Title_case)): + - Capitalize the first and last words of titles and subtitles. + - Capitalize "major" words (nouns, pronouns, verbs, adjectives, adverbs, and some conjunctions). + - Lowercase the conjunctions _and_, _but_, _for_, _or_, and _nor_. + - Lowercase the articles _the_, _a_, and _an_. + - Lowercase prepositions, regardless of length, except when they are stressed, are used adverbially or adjectivally, or are used as conjunctions. + - Lowercase the words _to_ and _as_. + - Lowercase the second part of Latin species names. +4. **Sentence-case string**: a non-blank string that follows the below guidelines (see https://en.wikipedia.org/wiki/Letter_case#Sentence_case): + - Capitalize the first word of the sentence, as well as proper nouns and other words as required by a more specific rule. +5. **Valid `files` pattern**: A non-blank string¹ that specifies a location of a file used in an exercise, relative to the exercise's directory. A pattern may use one of the following placeholders: - `%{kebab_slug}`: the `kebab-case` exercise slug (e.g. `bit-manipulation`) - `%{snake_slug}`: the `snake_case` exercise slug (e.g. `bit_manipulation`) - `%{camel_slug}`: the `camelCase` exercise slug (e.g. `bitManipulation`) - `%{pascal_slug}`: the `PascalCase` exercise slug (e.g. `BitManipulation`) -6. Unique version 4 UUID string: A string that satisfies all of these conditions: +6. **Unique version 4 UUID string**: A string that satisfies all of these conditions: - It only exists once in the track-level `config.json` file of a given Exercism track - It does not exist in the track-level `config.json` file of any other Exercism track - It does not exist in any `canonical-data.json` file in https://github.com/exercism/problem-specifications diff --git a/building/github/contributors-pull-request-guide.md b/building/github/contributors-pull-request-guide.md index fd3075a6..cb91c230 100644 --- a/building/github/contributors-pull-request-guide.md +++ b/building/github/contributors-pull-request-guide.md @@ -1,33 +1,30 @@ # The Contributors' Guide to Pull Requests Thank you for wanting to contribute to Exercism! -Before creating a pull request, please read the [how to make a great Pull Request][how-to-make-a-great-pr] guide which aims to help you with the non-technical side of pull requests so that you have a great experience and help our maintaining team at the same time. +Before creating a pull request, please read the [how to make a great pull request][how-to-make-a-great-pr] guide which aims to help you with the non-technical side of pull requests so that you have a great experience and help our maintaining team at the same time. This documents contains some additional, Exercism-specific guidelines for different types of pull requests. ## Exercise Pull Requests -Creating a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercises. -For this reason, it can take a maintainer 2 to 3 hours to review and can result in dozens of comments. - -To help you, there'll be one primary reviewer commenting on your pull request. +Creating a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercises. For this reason, it can take a maintainer two to three hours to review and can result in dozens of comments. To help you, there'll be one primary reviewer commenting on your pull request. Don't be discouraged by the number of review comments; it is perfectly normal for an exercise to go through several rewrites before arriving at something both you and the primary reviewer are happy with. Here are some recommendations to help streamline the process even more. -### Recommendation: read the documentation +### Read the documentation Before creating a pull request for an exercise, be sure to read the [concept exercise documentation][concept-exercises] respectively [practice exercise documentation][practice-exercises]. Reading the documentation beforehand helps immensely, as reviews will have less comments and your pull request will be merged much sooner. -### Recommendation: examine existing exercises (if any) +### Examine existing exercises (if any) If the track has any existing exercises, take the time to study them a bit to discover what they look like and how they're structured. ## Practice Exercise Pull Requests -### Recommendation: consider whether the change actually belongs in problems-specifications +### Consider whether the change actually belongs in problems-specifications Some of the contents of a Practice Exercise (such as its introduction) comes from its (shared) metadata as defined in the [problems-specifications repo][problem-specifications]. If you're intending to update such content, consider whether the change might benefit other tracks too. @@ -35,7 +32,7 @@ If so, please open a pull request to the exercise's metadata in the [problems-sp ## General recommendations -### Recommendation: refrain from doing unsolicited reviews +### Refrain from doing unsolicited reviews All pull request reviews are done by one (or more) maintainers of the track, as they are responsible for signing off all changes to the repository. Maintainers doing the review also helps to avoid conflicting feedback for the pull request author. diff --git a/building/github/gha-best-practices.md b/building/github/gha-best-practices.md index fbba15ae..e1018abb 100644 --- a/building/github/gha-best-practices.md +++ b/building/github/gha-best-practices.md @@ -41,16 +41,16 @@ jobs: ### Consider if (third-party) actions are really needed -Actions should be treated like dependencies in your favourite programming language[^1], they are code written by third party authors outside of the control of Exercism. +Actions should be treated like dependencies in your favourite programming language 1, they are code written by third party authors outside of the control of Exercism. Even if you trust the authors of the action, there may be a hostile takeover of the repository which will indirectly give those people access to Exercism repos, including write access. Therefore, you should carefully consider if introducing a new action is really worth it or if it's better to move the code into a (new) action under Exercism's control. Also consider if the action is actively maintained, e.g. by checking recent repo activity or ensuring that the action is part of an organisation rather than an individual account. -Actions published by [GitHub](https://github.com/actions/) or the Exercism org can generally be considered as safe(ish) to include without special consideration. +Actions published by [GitHub](https://github.com/actions/) or the Exercism organization can generally be considered as safe(ish) to include without special consideration. -[^1]: unless the language uses the npm ecosystem. +1: unless the language uses the `npm` ecosystem. ### Limit scope of workflow token diff --git a/building/github/maintainers-pull-request-guide.md b/building/github/maintainers-pull-request-guide.md index 4e4c8a1c..0a29651b 100644 --- a/building/github/maintainers-pull-request-guide.md +++ b/building/github/maintainers-pull-request-guide.md @@ -6,7 +6,7 @@ This document contains some Exercism-specific pull request review guidelines. ## Reviewing Exercise Pull Requests Reviewing a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercise. -For this reason, a first-pass review by a maintainer often take 2 to 3 hours and results in dozens of comments. +For this reason, a first-pass review by a maintainer often takes two to three hours and results in dozens of comments. For Concept Exercises, there are also files with similar goals/contents (e.g. the exercise and concept introduction), where focusing on getting one of those perfect first is essential before branching out too far. To help streamline this workflow, we've developed the following recommendations. diff --git a/building/tooling/analyzers/docker.md b/building/tooling/analyzers/docker.md index c6872b83..71a357a0 100644 --- a/building/tooling/analyzers/docker.md +++ b/building/tooling/analyzers/docker.md @@ -2,7 +2,7 @@ Our Analyzers are deployed as Docker images. -Please read the [general Tooling docker information](/docs/building/tooling/analyzers/docker) to familiarize yourself with how these work. +Please read the [general tooling Docker information](/docs/building/tooling/docker) to familiarize yourself with how these work. When we run the Analyzer's container we execute a `run.sh` script. To ensure this works properly the following rules must be following: diff --git a/building/tooling/analyzers/feedback-guidelines.md b/building/tooling/analyzers/feedback-guidelines.md index bb9dd2df..232af7d7 100644 --- a/building/tooling/analyzers/feedback-guidelines.md +++ b/building/tooling/analyzers/feedback-guidelines.md @@ -4,8 +4,8 @@ _NOTE: This spec is currently being updated._ The goal of a language track on Exercism is to give people a way to achieve a high level of [fluency](/docs/using/product/fluency) at a low level of proficiency. We're aiming for fluency -in the syntax, idioms, and the standard library of the language. You can read -more about the [goal of exercism here](https://github.com/exercism/docs/blob/master/about/goal-of-exercism.md). +in the syntax, idioms, and the standard library of the language. Read +more about the [goal of Exercism here](https://github.com/exercism/docs/blob/master/about/goal-of-exercism.md). ## Definitions From 06c7fd5e7b1aa71d4bebf889a64efada38f185fc Mon Sep 17 00:00:00 2001 From: Safwan Samsudeen Date: Tue, 21 Mar 2023 19:21:21 +0530 Subject: [PATCH 2/5] revert changes based on review --- .gitignore | 3 +- building/configlet/lint.md | 8 +++-- .../github/contributors-pull-request-guide.md | 3 +- building/github/gha-best-practices.md | 30 +++++++++++-------- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/.gitignore b/.gitignore index 2752eb92..40b878db 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1 @@ -node_modules/ -.DS_Store +node_modules/ \ No newline at end of file diff --git a/building/configlet/lint.md b/building/configlet/lint.md index 89557b15..20ffdbf1 100644 --- a/building/configlet/lint.md +++ b/building/configlet/lint.md @@ -527,8 +527,8 @@ The `config.json` file should have the following checks: ## Glossary 1. **Non-blank string**: a string that contains at least one non-whitespace character. -2. **Kebab-case string**: a string that contains only characters in the range `[a-z0-9]`, optionally separated by dashes (e.g. "two-fer"). It must match the regular expression: `^[a-z0-9]+(-[a-z0-9]+)*$` -3. **Title-case string**: a non-blank string that follows the below guidelines (from the [Chicago Manual of Style](https://en.wikipedia.org/wiki/Title_case)): +2. **kebab-case string**: a string that contains only characters in the range `[a-z0-9]`, optionally separated by dashes (e.g. "two-fer"). It must match the regular expression: `^[a-z0-9]+(-[a-z0-9]+)*$` +3. **Title Case string**: a non-blank string that follows the below guidelines (from the [Chicago Manual of Style](https://en.wikipedia.org/wiki/Title_case#Chicago%20Manual%20of%20Style:~:text=Guitar\).%5B3%5D-,Chicago%20Manual%20of%20Style,-%5Bedit%5D) of title case): - Capitalize the first and last words of titles and subtitles. - Capitalize "major" words (nouns, pronouns, verbs, adjectives, adverbs, and some conjunctions). - Lowercase the conjunctions _and_, _but_, _for_, _or_, and _nor_. @@ -536,14 +536,16 @@ The `config.json` file should have the following checks: - Lowercase prepositions, regardless of length, except when they are stressed, are used adverbially or adjectivally, or are used as conjunctions. - Lowercase the words _to_ and _as_. - Lowercase the second part of Latin species names. -4. **Sentence-case string**: a non-blank string that follows the below guidelines (see https://en.wikipedia.org/wiki/Letter_case#Sentence_case): +4. **Sentence Case string**: a non-blank string that follows the below [guidelines](https://en.wikipedia.org/wiki/Letter_case#Sentence_case): - Capitalize the first word of the sentence, as well as proper nouns and other words as required by a more specific rule. 5. **Valid `files` pattern**: A non-blank string¹ that specifies a location of a file used in an exercise, relative to the exercise's directory. A pattern may use one of the following placeholders: + - `%{kebab_slug}`: the `kebab-case` exercise slug (e.g. `bit-manipulation`) - `%{snake_slug}`: the `snake_case` exercise slug (e.g. `bit_manipulation`) - `%{camel_slug}`: the `camelCase` exercise slug (e.g. `bitManipulation`) - `%{pascal_slug}`: the `PascalCase` exercise slug (e.g. `BitManipulation`) 6. **Unique version 4 UUID string**: A string that satisfies all of these conditions: + - It only exists once in the track-level `config.json` file of a given Exercism track - It does not exist in the track-level `config.json` file of any other Exercism track - It does not exist in any `canonical-data.json` file in https://github.com/exercism/problem-specifications diff --git a/building/github/contributors-pull-request-guide.md b/building/github/contributors-pull-request-guide.md index cb91c230..ac8f7cf9 100644 --- a/building/github/contributors-pull-request-guide.md +++ b/building/github/contributors-pull-request-guide.md @@ -7,7 +7,8 @@ This documents contains some additional, Exercism-specific guidelines for differ ## Exercise Pull Requests -Creating a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercises. For this reason, it can take a maintainer two to three hours to review and can result in dozens of comments. To help you, there'll be one primary reviewer commenting on your pull request. +Creating a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercises. For this reason, it can take a maintainer two to three hours to review and can result in dozens of comments. +To help you, there'll be one primary reviewer commenting on your pull request. Don't be discouraged by the number of review comments; it is perfectly normal for an exercise to go through several rewrites before arriving at something both you and the primary reviewer are happy with. diff --git a/building/github/gha-best-practices.md b/building/github/gha-best-practices.md index e1018abb..84b895f1 100644 --- a/building/github/gha-best-practices.md +++ b/building/github/gha-best-practices.md @@ -3,15 +3,21 @@ This _working_ document serves as a collection of best practices for making use of GitHub Actions. If you have any suggestions or additions, [please open a pull request on GitHub!](https://github.com/exercism/docs/edit/main/building/github/gha-best-practices.md) -- [Collection of Best Practices](#collection-of-best-practices) - - [Set timeouts for workflows](#set-timeouts-for-workflows) - - [Consider if (third-party) actions are really needed](#consider-if-third-party-actions-are-really-needed) - - [Limit scope of workflow token](#limit-scope-of-workflow-token) - - [Pin actions to SHAs](#pin-actions-to-shas) - - [Consider setting up a concurrency strategy](#consider-setting-up-a-concurrency-strategy) - - [Consider which triggers are really needed](#consider-which-triggers-are-really-needed) - - [Read the "Security hardening for GitHub Actions" guide](#read-the-security-hardening-for-github-actions-guide) -- [Workflow Checklist](#workflow-checklist) +- [GitHub Actions: Best Practices](#github-actions-best-practices) + - [Collection of Best Practices](#collection-of-best-practices) + - [Set timeouts for workflows](#set-timeouts-for-workflows) + - [Example](#example) + - [Consider if (third-party) actions are really needed](#consider-if-third-party-actions-are-really-needed) + - [Limit scope of workflow token](#limit-scope-of-workflow-token) + - [Example](#example-1) + - [Pin actions to SHAs](#pin-actions-to-shas) + - [Finding the commit SHA](#finding-the-commit-sha) + - [Example](#example-2) + - [Consider setting up a concurrency strategy](#consider-setting-up-a-concurrency-strategy) + - [Example](#example-3) + - [Consider which triggers are really needed](#consider-which-triggers-are-really-needed) + - [Read the "Security hardening for GitHub Actions" guide](#read-the-security-hardening-for-github-actions-guide) + - [Workflow Checklist](#workflow-checklist) ## Collection of Best Practices @@ -41,16 +47,16 @@ jobs: ### Consider if (third-party) actions are really needed -Actions should be treated like dependencies in your favourite programming language 1, they are code written by third party authors outside of the control of Exercism. +Actions should be treated like dependencies in your favourite programming language[^1], they are code written by third party authors outside of the control of Exercism. Even if you trust the authors of the action, there may be a hostile takeover of the repository which will indirectly give those people access to Exercism repos, including write access. Therefore, you should carefully consider if introducing a new action is really worth it or if it's better to move the code into a (new) action under Exercism's control. Also consider if the action is actively maintained, e.g. by checking recent repo activity or ensuring that the action is part of an organisation rather than an individual account. -Actions published by [GitHub](https://github.com/actions/) or the Exercism organization can generally be considered as safe(ish) to include without special consideration. +Actions published by [GitHub](https://github.com/actions/) or the Exercism org can generally be considered as safe(ish) to include without special consideration. -1: unless the language uses the `npm` ecosystem. +[^1]: unless the language uses the npm ecosystem. ### Limit scope of workflow token From 63207f55de4d7ae34fd371256ff1279419384385 Mon Sep 17 00:00:00 2001 From: Safwan Samsudeen Date: Tue, 11 Apr 2023 19:52:08 +0530 Subject: [PATCH 3/5] change based on ee7's review --- .gitignore | 2 +- building/github/contributors-pull-request-guide.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 40b878db..c2658d7d 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1 @@ -node_modules/ \ No newline at end of file +node_modules/ diff --git a/building/github/contributors-pull-request-guide.md b/building/github/contributors-pull-request-guide.md index ac8f7cf9..fbb5a0ba 100644 --- a/building/github/contributors-pull-request-guide.md +++ b/building/github/contributors-pull-request-guide.md @@ -7,7 +7,8 @@ This documents contains some additional, Exercism-specific guidelines for differ ## Exercise Pull Requests -Creating a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercises. For this reason, it can take a maintainer two to three hours to review and can result in dozens of comments. +Creating a pull request for a Concept Exercise or Practice Exercise can be daunting with the many rules around these types of exercises. +For this reason, it can take a maintainer two to three hours to review and can result in dozens of comments. To help you, there'll be one primary reviewer commenting on your pull request. Don't be discouraged by the number of review comments; it is perfectly normal for an exercise to go through several rewrites before arriving at something both you and the primary reviewer are happy with. From f70142ee5edcdc4831a0a15139ea5dd73e57b7a8 Mon Sep 17 00:00:00 2001 From: Safwan Samsudeen Date: Wed, 12 Apr 2023 05:09:39 +0530 Subject: [PATCH 4/5] change url based on ee7's review --- building/configlet/lint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/building/configlet/lint.md b/building/configlet/lint.md index 20ffdbf1..07affb2a 100644 --- a/building/configlet/lint.md +++ b/building/configlet/lint.md @@ -528,7 +528,7 @@ The `config.json` file should have the following checks: 1. **Non-blank string**: a string that contains at least one non-whitespace character. 2. **kebab-case string**: a string that contains only characters in the range `[a-z0-9]`, optionally separated by dashes (e.g. "two-fer"). It must match the regular expression: `^[a-z0-9]+(-[a-z0-9]+)*$` -3. **Title Case string**: a non-blank string that follows the below guidelines (from the [Chicago Manual of Style](https://en.wikipedia.org/wiki/Title_case#Chicago%20Manual%20of%20Style:~:text=Guitar\).%5B3%5D-,Chicago%20Manual%20of%20Style,-%5Bedit%5D) of title case): +3. **Title Case string**: a non-blank string that follows the below [guidelines from the Chicago Manual of Style](https://en.wikipedia.org/wiki/Title_case#Chicago_Manual_of_Style): - Capitalize the first and last words of titles and subtitles. - Capitalize "major" words (nouns, pronouns, verbs, adjectives, adverbs, and some conjunctions). - Lowercase the conjunctions _and_, _but_, _for_, _or_, and _nor_. From b2eea33c8f4272c460467b4bdc6ad47e10694d8c Mon Sep 17 00:00:00 2001 From: Safwan Samsudeen <62411302+safwansamsudeen@users.noreply.github.com> Date: Fri, 28 Apr 2023 16:50:11 +0530 Subject: [PATCH 5/5] Update pull-request-guide on erik's review Co-authored-by: Erik Schierboom --- building/github/contributors-pull-request-guide.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/building/github/contributors-pull-request-guide.md b/building/github/contributors-pull-request-guide.md index fbb5a0ba..b1512eb3 100644 --- a/building/github/contributors-pull-request-guide.md +++ b/building/github/contributors-pull-request-guide.md @@ -13,6 +13,8 @@ To help you, there'll be one primary reviewer commenting on your pull request. Don't be discouraged by the number of review comments; it is perfectly normal for an exercise to go through several rewrites before arriving at something both you and the primary reviewer are happy with. +## Recommendations + Here are some recommendations to help streamline the process even more. ### Read the documentation