This repository has been archived by the owner on Mar 13, 2022. It is now read-only.
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@UziTech
Ill start with most relevant: pls accept the org invite, I resent invite just now, when I realized that original invite was for wrong place adn I cant convert to team member.
But here is the stuff RE: this commit.
Do we want a package-lock.json? Its on .gitignore and I could add it to .npmrc as false.
Re: changelog.md
This autogenerated is a bit rubbish as is,, only adds the entry which follows the quirky syntax for semantic release.
it ignores everything else, and its important this, because I dont want to trigger releases at every commit thats pushed to master either so cant prefix everything with fixed if for instance its meta stuff, and since its not a commit starting with
fix
its not going into changelog.. would entries with chore go in?Generally I dont find this OK, prefer no changelog to this. Unless this can be configured to allow all other entries since last tag/release
and finally.
So husky seems like a good idea, but I dont commit anything command line, I use GitHub/Git Atom addon, whic means IDK if husky is
I received several of these messages below in a warning popup.
Its a little beyond getting used to a new way of doing things =)
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely no package-lock.json
way too much noise for a dependency of a dependency/
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should commit the package-lock.json file. At best it will prevent weird errors on users machine that are hard to reproduce on our machine. At worst it is just a file that we need to make sure is updated whenever dependencies are updated.
Example:
"underscore": "^1.9.1"
The benefit of package-lock.json is to ensure that everyone with the same version of terminus will have the same version of the dependencies.
semantic release automates a few things:
The idea is that all you have to worry about is creating relevant commit messages and semantic release will handle putting them in the right place.
We could configure it to only release on a different branch. If we create a
"release"
branch and configure semantic release to use that branch we would just need to merge master into that branch in order to release a new version. That way we could control how many fixes get into each release and we still get the benefits of automated release and changelog.It took me a while to get used to writing useful commit messages as well but I can say that it has helped in many ways, not least of which is trying to remember what changes were made each time I would release a new version.
I don't use command line either. The addon probably is using command line so as long as you have husky installed as a devdependency it should be working.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to remove semantic release I would be ok with that, but I think we should have a changelog. Change logs make it much easier to track down bugs than looking through commit logs.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the noise on GitHub is impossible with the security alerts in this case for jquery 3.4.0, we dont use that dep, some dep we use has that jquery dep, we cant fix it directly, and although I can simply dismiss the security warning n GitHub, I rather not., that means I dismiss all security warnings not just inclusive to the jquery dep issue.
Its disabled for now, because of that. its not ideal and I fully agree with you on the bug report front.
Ill look into re-enabling without the grief of noise.
for that we would work in other branches and push to master, I work mostly ontop of master, creating branches for each type of work, its not practical for me as Im testing in macOS/Linux/Windows theres not enough time in the day.
You made the point well the other day, I wouldnt want anyone to be deprived of being able to make a release either, so long as everyone still can thats the important thing.
Re: changelogs.
I also like changelogs and I dont want to manually maintain one either, I like them especially if they dont require knowing any specific keywords and every commit is counted and added irrespective of wordage. Makes sense?
So how do we proceed, direct everyone that makes a contrib to all keywords in a CONTRIBUTING.MD? or is there a way to configure the changelog gen?
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if there is a way to configure semantic release to add every commit to the changelog. It seems like that might make the changelog harder to read since some changes (like readme or ci changes) aren't really relevant. I think it would be best to point people to a CONTRIBUTING.md file with the correct syntax.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its says here https://github.com/conventional-changelog/commitlint/#what-is-commitlint keywords
also https://github.com/karma-runner/karma/blob/master/CHANGELOG.md I like that format, easy to read and other entries are added.
All work is relevant, but specific stuff is more important. IDK what would be hard to read, unless its 100 change entries and 5 or 6 are fixes/feat/other dotted all over, but the format of the changelog above it makes things easy and separate.
Just unsure how to configure the conventional changelog via semantic version, didnt look into that, or if its as easy as adding a specific config file.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like
karma
is using the same as the default configuration for semantic release:They are only adding certain commits to the changelog based on the type of commit. (
fix:
goes under "Bug Fixes",feat:
goes under "Features", etc.)The commit types are found here: Git Commit Guidelines
As long as we stick to those types semantic release will put them in the correct place in the commit log and release log.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets go with that.
if we want to include the legacy changelog also or not thats another matter.
Once thats done I add the missing entries manually once and hope nothing gets overwritten after.
Ill try to whip out a CONTRIBUTING.MDbut I wanted to know coding conventions etc/formatting (this is why I also wanted prettier to extend eslint and do the formatting for us.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Atom use prettier
https://github.com/atom/atom/search?q=prettier&unscoped_q=prettier
e.g commit
atom/atom@e213a69
If anyone has a preference on settings for that, or use what they use or some standard, idk.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure how prettier is different than
eslint --fix
it seems like it is just another config file we need to maintain.15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a set and forget type thing. But theres eslint rules that may need to be disabled so theres no conflicts (according to their docs),
https://prettier.io/docs/en/comparison.html
And while eslint may enforce some style guide its not a complete solution. Not all rules can be autofixed in eslint.
Lets say we have people pushing contribs, they didnt read the CONTRIBUTING.MD or ignored it, they submit pull request and the linter/prettier fix the guide to conform with our styleguide in its full glory.
Sadly that only works for code, commit messages is a whole diff business to enforce.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@commitlint/travis-cli
can be used to check each commit in a PR.15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check? We want to check and fix no? Probably not possible for messages. Anywho looks good.
So now we have a convention for messages we need one for code style. Ive no idea what to add to the CONTRIBUTING.MD under that heading.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would be possible to automatically determine what the commit message should be.
Standard seems to be pretty common. I don't think it matters too much what we pick as long as there is a definite style. I usually just stick with the defaults or a common style since they are well documented.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes fully agree,! So lets enforce that via standard with --fix.
Iike this it says on their site.
This is precisely why I think this is a good idea. 👍 😃
Would this work besides eslint its not clear.?
So I know what to what in contributing.md now =)
Also, I finally warmed to the semantic release idea, if we conform to angular standard commit messages I think these are all added to changelog in their respective areas if we use the this changelog format
Ill get started on the contributing.md ;)
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a PR #13 with the
standard
configuration for eslint. The way it is set up right now it will just check to see if everything is formatted correctly in a PR. The author will have to runnpm run fix
to actually fix any linting errors.15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I guess this cant be autofxed after merge via travis. Looks good I guess.
15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could run a
fix
step on the master branch whenever a PR is merged but in the past I have run into weird issues. It just seems easier to tell them to runnpm run fix
when submitting a PR. Plus then we don't have to read through ugly code when reviewing a PR.15ae22a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, indeed.