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.

Sign up for GitHub

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

Jump to bottom

Feat: recover the complete changelog.md #10

Merged
merged 15 commits into from
Sep 13, 2017
Merged

Feat: recover the complete changelog.md #10

merged 15 commits into from
Sep 13, 2017

Conversation

aichbauer
Copy link
Owner

No description provided.

@coveralls
Copy link

coveralls commented Sep 3, 2017 •
edited

Coverage Status

Coverage decreased (-4.4%) to 91.875% when pulling 2a5eb7c on feat/completeChangelog into 34e94ff on master.

@coveralls
Copy link

coveralls commented Sep 6, 2017 •
edited

Coverage Status

Coverage increased (+0.5%) to 96.774% when pulling ac04441 on feat/completeChangelog into 34e94ff on master.

@coveralls
Copy link

coveralls commented Sep 7, 2017 •
edited

Coverage Status

Coverage increased (+0.5%) to 96.774% when pulling 8402b76 on feat/completeChangelog into 82d1fcc on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 96.774% when pulling a60981a on feat/completeChangelog into 34e94ff on master.

@coveralls
Copy link

coveralls commented Sep 7, 2017 •
edited

Coverage Status

Coverage increased (+0.5%) to 96.774% when pulling 8884e32 on feat/completeChangelog into 82d1fcc on master.

JPeer264
JPeer264 requested changes Sep 7, 2017
View reviewed changes
Copy link
Collaborator

@JPeer264 JPeer264 left a comment

Choose a reason for hiding this comment

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

What if a changelog exists (maybe used sgr) and then I forgot to release 2 times with sgr but then I want to recover those missing two. It won't work right?

lib/cli.js Outdated
@@ -15,13 +15,16 @@ import getLatestVersion from './helpers/getLatestVersion';
import pkg from '../package.json';
import questions from './questions';
import tasks from './tasks';
import recoverTasks from './recover-tasks/index';
Copy link
Collaborator

Choose a reason for hiding this comment

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

/index is not necessary

lib/helpers/generateCompleteChangelog.js Outdated
const tagsCount = countGitTags();
const tags = taggedCommits({
path: cwd,
lookBehind: tagsCount,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it would be more performant if you use Number.POSITIVE_INFINITE

lib/helpers/generateCompleteChangelog.js Outdated
commit: tag.hash,
}).date;

changelogData = `${tag.version} - ${moment(tagDate, 'ddd MMM D HH:mm:ss YYYY Z').format('MMMM, DD YYYY')}\n`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should make a global formating, since it is also here used (just with the moment() param diff):

node-semantic-git-release-cli/lib/helpers/updateChangelog.js

Line 20 in 82d1fcc

stream.write(`${version} - ${moment().format('MMMM, DD YYYY')}\n\n`);

But if we want to change it at some point there is just one place for it.

lib/helpers/generateCompleteChangelog.js Outdated
return;
}

changelogData = `${changelogData} * ${commitInfo.shortHash} ${commitInfo.message.split('\n')[0]} (${commitInfo.author})\n`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe same here as with the header:

node-semantic-git-release-cli/lib/helpers/updateChangelog.js

Line 25 in 82d1fcc

stream.write(`* ${commitInfo.shortHash} ${commitInfo.message.split('\n')[0]} (${commitInfo.author})\n`);

Could be a global function where we pass in the entire commitInfo as param. So we can vary better then (if something changes).

test/lib-helpers/generateCompleteChangelog.js Outdated
@@ -0,0 +1,94 @@
import fs from 'fs';
import fse from 'fs-extra';
Copy link
Collaborator

Choose a reason for hiding this comment

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

import fse is not quite needed. You can actually write:

import fs from 'fs-extra';

@coveralls
Copy link

coveralls commented Sep 7, 2017 •
edited

Coverage Status

Coverage increased (+1.1%) to 97.436% when pulling b1cce8f on feat/completeChangelog into 82d1fcc on master.

@coveralls
Copy link

coveralls commented Sep 7, 2017 •
edited

Coverage Status

Coverage increased (+1.1%) to 97.436% when pulling 1af8ba6 on feat/completeChangelog into 82d1fcc on master.

@aichbauer
Copy link
Owner Author

@JPeer264 why shouldn't you not be able to recover those two releases? The changelog gets rewritten everytime you hit sgr -r. Which means if you have tagged your versions even if you didn't used sgr you can recover those two releases. I do not see any problems there. Can you show me the part in the code, why you think this is not possible?

@aichbauer aichbauer merged commit 67e04d0 into master Sep 13, 2017
@aichbauer aichbauer deleted the feat/completeChangelog branch September 13, 2017 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JPeer264 JPeer264 JPeer264 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@aichbauer @coveralls @JPeer264