Skip to content

Propose maintinaing a contributors.md file to manage access changes #61

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.

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

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

goofballLogic
Copy link
Member

No description provided.

@goofballLogic goofballLogic requested a review from asbjornu July 2, 2020 11:53
@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

Merging #61 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #61   +/-   ##
=======================================
  Coverage   68.59%   68.59%           
=======================================
  Files          22       22           
  Lines        4053     4053           
  Branches     1028     1028           
=======================================
  Hits         2780     2780           
  Misses       1130     1130           
  Partials      143      143           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7202bc6...cea302b. Read the comment docs.

@asbjornu
Copy link
Member

asbjornu commented Jul 2, 2020

This is a nice addition, but perhaps we can add it as the special CODEOWNERS file instead, as that is understood by GitHub and serves multiple purposes?

@asbjornu asbjornu mentioned this pull request Jul 2, 2020
@goofballLogic
Copy link
Member Author

I think CODEOWNERS can only contain default reviewers (which we also need)?

I was hoping for a way manage suggesting new contributors via consensus. For example, yesterday I added @wattsm based on my personal knowledge of Mark's prowess with .Net, but there wasn't an easy way for me to run this past you for agreement. This file would allow me to do a PR with his name, and ask for your sign off before proceeding.

Or, I could be overthinking it....

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

What do you think about adding contributor's-img as well?

@goofballLogic goofballLogic requested a review from asbjornu July 4, 2020 13:35
@goofballLogic goofballLogic requested a review from asbjornu July 5, 2020 14:33
asbjornu
asbjornu previously approved these changes Jul 5, 2020
@goofballLogic
Copy link
Member Author

@asbjornu do you know why we don't have Squash option enabled for this repo?

@asbjornu
Copy link
Member

asbjornu commented Jul 7, 2020

@goofballLogic, I've disabled it. I'm very strongly against everything GitHub offers in terms of updating branches, squashing and merging. Everything they do and enable in the UI, I basically like the opposite. 😄

Let's take squash merge as an example. The biggest problem with it, imho, is:

Pull requests with squashed commits are merged using the fast-forward option.

I like the explicit notion of a non-fast-forward merge-commit. I also like being able to go through the individual commits in a branch to understand the thought process behind a feature. With fast-forward squash merges, a lot of information is lost.

I'm also against GitHub's "update branch" button, since it doesn't perform a rebase against the target branch, but instead merges it. I suppose this goes well with squash merge, since then all those merge-commits disappear, but without squashing, they make the log history very cluttered.

The way I like to circumvent all of this is to manually rebase, often interactively, to make the commit history in a branch sensible and uncluttered.

@goofballLogic
Copy link
Member Author

ok well i have to disagree but I'm happy to rebase myself and you'll then need to reapprove

@goofballLogic
Copy link
Member Author

At some stage we should perhaps do an audit of the settings we prefer for the repository so you and I (at least) are in sync on this stuff.

@goofballLogic goofballLogic force-pushed the goofballLogic-contributors-md branch from 0603322 to cea302b Compare July 7, 2020 19:34
@goofballLogic goofballLogic requested a review from asbjornu July 7, 2020 19:35
@goofballLogic goofballLogic self-assigned this Jul 7, 2020
@asbjornu
Copy link
Member

asbjornu commented Jul 7, 2020

At some stage we should perhaps do an audit of the settings we prefer for the repository so you and I (at least) are in sync on this stuff.

Definitely! I'm not fuzzy about it, I've just set it up according to my defaults and preference.

@goofballLogic
Copy link
Member Author

This will need to be re-approved now I squashed the commits manually

@goofballLogic goofballLogic merged commit a98b12f into master Jul 9, 2020
@goofballLogic goofballLogic deleted the goofballLogic-contributors-md branch July 9, 2020 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants