Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
create a readme for draft logs - throw an error when skipping bad fil…
…enames
  • Loading branch information
archmoj committed Jun 30, 2021
commit 3a91c0e380886d6292b12a71b22fd25d298b5f82
1 change: 0 additions & 1 deletion draftlogs/.gitignore

This file was deleted.

22 changes: 22 additions & 0 deletions draftlogs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## Directory of draft logs to help prepare the upcoming [CHANGELOG](https://github.com/plotly/plotly.js/blob/master/CHANGELOG.md)

It is required that the PR contributors start the filename with a number.
This number should preferably be the PR number.
The number of issue they are trying to close could also be used.
The markdown file should end with one of the followings:
1. `_fix.md` to propose a bug fix
2. `_add.md` to propose new features
3. `_remove.md` to propose a feature removal
4. `_change.md` to propose a minor/major change
5. `_deprecate.md` to propose a feature deprecate

### Example filename and content for PR numbered 5546 for adding a new feature
- filename: `5546_add.md`
- content:
```
- Add `icicle` trace type [[#5546](https://github.com/plotly/plotly.js/pull/5546)]
```
which would render
- Add `icicle` trace type [[#5546](https://github.com/plotly/plotly.js/pull/5546)]

> Please start your single-line or multiple lined message with a verb. You could basically use the PR description while providing a link to the PR similar to the above example is appreciated too.
9 changes: 9 additions & 0 deletions tasks/use_draftlogs.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var all = {

var ENTER = '\n';

var skippedFiles = [];
for(var i = 0; i < len; i++) {
var filename = allLogs[i];
var message = fs.readFileSync(path.join(pathToDraftlogs, filename), { encoding: 'utf-8' }).toString();
Expand All @@ -49,6 +50,8 @@ for(var i = 0; i < len; i++) {
all.Changed.push(message);
} else if(filename.endsWith('_fix.md')) {
all.Fixed.push(message);
} else {
skippedFiles.push(filename);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Else throw an error, logging the filename as having been ignored.

Might want to also loosen the endsWith tests - to eg filename.toLowerCase().indexOf('_add') !== -1 so stuff like 5678_Added.md will still be picked up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we have a test which disallows use of uppercase in filenames.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We do test for uppercase, but I don't think it'll dig into the draftlogs dir. And there's still add vs adds vs added etc. Anyway the main thing is to throw an error if we ignore the file. After adding that if you still want to be strict about names that's ok, it'll just occasionally make more work during release if we get an incorrect name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Addressed in 3a91c0e.

}

Expand All @@ -75,3 +78,9 @@ append('Fixed');
draftNewChangelog.push(foot);

fs.writeFileSync(pathToChangelog, draftNewChangelog.join(ENTER), { encoding: 'utf-8' });

if(skippedFiles.length) {
throw JSON.stringify({
'skippedFiles': skippedFiles
}, null, 2);
}