diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 68e551146..441c88ae1 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -70,6 +70,11 @@ Don't: "
Another paragraph, maybe in a different style, e.g. italics or bold."))); ``` +# TODO's +In Mudlet's decade of existence, TODO's get added to the codebase but almost never resolved. Having them in the codebase does not add any value if they are just sitting there and not acted upon. This raises the question, why have them around at all if they are not adding any value? To this effect, the project decided not to add future TODOs to the code and work on removing existing ones. + +In case the TODO is an improvement, a pull request implementing the idea or an issue suggesting it can be added - no TODO necessary. In case it's a legitimate problem, it's better to fix it right away instead of kicking the can down the road. + # Git commit guidelines for core team ## Refactoring @@ -83,11 +88,6 @@ Don't: * PR Title must start with `fix`, `improve`, `add`, or `infra` * This facilitates automatic changelog gathering and categorization * Cannot merge until it is fixed: core team can always adjust it before merging -* Any new TODO in a source file must have a Mudlet github issue on the same line - * good: - * //TODO: https://github.com/Mudlet/Mudlet/issues/1234 - * bad: - * //TODO: a thing we aren't actually tracking with an issue Danger will also give a heads up if the PR title is long, or if more than 10 source files are changed in a single PR. These are not blocked but the warnings should serve to draw attention to something which may require a double check. More info below. diff --git a/dangerfile.js b/dangerfile.js index d8d65b792..ecf52463a 100644 --- a/dangerfile.js +++ b/dangerfile.js @@ -1,6 +1,4 @@ const {danger, fail, message, warn} = require('danger'); -const ISSUE_REGEX = /https?:\/\/(?:www\.)?github\.com\/Mudlet\/Mudlet\/issues\/(\d+)/i -const ISSUE_URL = "https://github.com/Mudlet/Mudlet/issues" const SOURCE_REGEX = /.*\.(cpp|c|h|lua)$/i const TITLE_REGEX = /^(fix|improve|add|infra)/i const touched_files = [...danger.git.created_files, ...danger.git.modified_files] @@ -21,20 +19,11 @@ if (title_type) { sourcefiles.forEach(function(filename) { const additions = danger.git.diffForFile(filename) additions.then(diff => { - const issues = [] diff.added.split("\n").forEach(function(item) { if (item.includes("TODO:")) { - const has_issue = item.match(ISSUE_REGEX) - if (!has_issue) { - fail(`Source file ${filename} includes a TODO with no Mudlet issue link.\n New TODO items in source files must have an accompanying github issue`) - } else { - issues.push(has_issue[1]) - } + fail(`Source file ${filename} includes a TODO! Can you make the change right away? If no, better not to create a TODO - they just tend to hang around and never get fixed.`) } }) - if (issues.length > 0) { - message(`\`${filename}\` adds TODO issues: ${issues.map(iss => `[${iss}](${ISSUE_URL}/${iss})`).join(", ")}`,{icon: ":heavy_check_mark:"} ) - } }) })