-
Notifications
You must be signed in to change notification settings - Fork 39
2017Q1 release #237
2017Q1 release #237
Conversation
love the change log! cool! |
👍 Quarterly releases are fine, although releases every 6 month would also be sufficient. What I don't like is the new authors file. With the number of commits it feels like a competition which it shouldn't be. Why not just list all authors independent of their number of commits? If you really need that information you can extract it quite easily as shown by your script. |
Not that I'm aware of. Maybe using the Milestones can be useful, don't know. We are getting better with the PRs though, way more consistent, and usually have a summary in the merge commit anyways. So it's not too much work.
I'm gonna try quarterly releases for now and and see how that works. I think it fits nicely with the arm-none-eabi-gcc release schedule.
Ok, I'll just make it an alphabetical list. I didn't think much about it, I basically just used what came out of shortlog. But I'm clearly the king and other things egomaniacs say about themselves. ;-P |
There is a GitHub API. It's a bit slow but easy to use (with pygithub). This prints a list of merged pull requests:
|
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's nice to see a release, there are a few interesting changes. I might actually update xpcc in the project I worked on a while ago :)
I added a few comments, sorry for being a bit too perfectionist, feel free to ignore them :D
CHANGELOG.md
Outdated
chosen for their importance to xpcc users as judged by the maintainer. | ||
This means that not every small commit makes it in here. | ||
|
||
There are quarterly release summaries formattes as: |
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.
Typo: formattes -> formatted
CHANGELOG.md
Outdated
|
||
## 2017-04-04: 2017q1 release | ||
|
||
As this is our first offical release it covers the last 12 months of development. |
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.
Typo: offical -> official
CHANGELOG.md
Outdated
The `xpcc/architecture/util.hpp` macros have been renamed and | ||
properly documented. | ||
A new header `xpcc/architecture/legacy_macros.hpp` contains backwards | ||
compatible mappings for application code. All occurances of the non- |
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.
Typo: occurances -> occurrences
CHANGELOG.md
Outdated
|
||
Three heap algorithms can be chosen from using the xpcc parameters: | ||
|
||
- newlib's dlmalloc (default): Chooses largest continous heap from page table. |
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.
Typo: continous -> continuous
tools/authors.py
Outdated
# get the shortlog summary | ||
output = subprocess.Popen("git shortlog -sn", shell=True, stdout=subprocess.PIPE).stdout.read() | ||
# parse the shortlog | ||
for line in output.split("\n")[:-1]: |
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.
Maybe output.splitlines()
instead of output.split("\n")[:-1]
?
tools/authors.py
Outdated
} | ||
|
||
# cleaned up dictionary | ||
commit_count = {} |
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.
You could use commit_count = defaultdict(int)
here, remove the add_commits
function below and just use commit_count[author] += shortlog[slauthor]
to update the counts.
tools/authors.py
Outdated
# merge authors | ||
for slauthor in shortlog.keys(): | ||
for author, aliases in author_alias.items(): | ||
if any(name in slauthor for name in (aliases + [author])): |
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.
What's the reason for checking for substrings instead of equality here? If equality checking works, this can be simplified to if slauthor in aliases:
. No need to add author
to the list, that will be handled by the else clause.
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.
Substring allows me to be lazy and only specify part of the alias name. Adding [author]
here so I don't need to specify it again as an alias. Coz I'm a lazy 🐒.
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.
That doesn't seem to be too robust. What if someone else named Kevin (or any of the aliases) contributes? That would match the alias incorrectly.
As for [author]
: you wouldn't need to add the "canonical" name to the aliases, since if it is not found in the alias list, the for loop would fall over to the else branch, and add the author there (as for every non-aliased author).
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.
Good point about [author]
, I overread your first comment too :-(
Regarding aliases, I hope to control the aliases better in the future, a lot of them come from before xpcc had me as maintainer. There are still author emails if all else fails.
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.
Okay, in that case go ahead with the release, I don't intend to stall this further.
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.
Thanks for the review. I don't perceive this as stalling at all, I enjoy everyone reviewing my code ;-) I still have to update the changelog anyways…
Thanks to all contributors for this release, especially the 7 first-timers!
|
Updates the Readme, Author files and adds a Changelog.
We're doing quartely releases now. Cos we're awesome.
This closes #160.
Please review changelog format and comment on release cycle.
I'm kind of hoping that GitHub renderes the PR numbers and commit SHA1 as a link as it does in the comments, but maybe that doesn't work.
cc @strongly-typed @Sh4rK @dergraaf @cajt @ekiwi