Repo workflow - Githubissues

john-peterson commented 11 years ago

I'm keeping this issue open because the existence of this mirror for metadata housekeeping. Is contentious. I looked again in 2023 for a way to extract all or save all metadata of this repo and I can't see it maybe I'm blind

john-peterson commented 11 years ago

Code style

@Matt_P for god sakes, use {} [01:33] @Matt_P and brackets for for/if etc

the style manual doesn't require brackets

there's an opinion (expressed in f.e. the Python syntax) that indentation without brackets allow adequate readability

@Matt_P and if you really aren't going to use a local var atleast do:

Describe what you mean with a local var

@Matt_P auto usb_ptr = GetUsbPointer(); [01:35] @Matt_P for (unsigned int i = 0; i < usb_ptr->m_WiiMotes.size(); i++) [01:36] @Matt_P you dont have to use auto

What's the benefit of the local variable in that example

[01:56] <_RachelB> JPeterson: it's easier to read [01:58] @Matt_P it doesnt request it multiple times

What's the benefit with not requesting the value through the function multiple times?

@Matt_P not wasting cycles [02:14] @Matt_P compiler cant optimise that out [02:14] @Matt_P as getusbpointer could be doing something important @Matt_P it cant optimise it out incase the value is chnaged by somethign else

What's the cycle difference between calling GetUsbPointer once and three times

[02:16] @Matt_P JPeterson: about 4 cycles per call or something tiny

[02:52] <_RachelB> just post it somewhere, i've already started to fix some formatting...

[02:56] <_RachelB> JPeterson: http://pastie.org/private/iuosh7p3ssji2fgzr7dbaa

[02:58] <_RachelB> JPeterson: i changed formatting, and the size_t to u32 in the header (needed to be done, but i haven't yet to avoid breaking old save states)

"size_t size" is changed to "u32 size" (on one line)

[03:00] <_RachelB> JPeterson: mostly moved { to a new line

The { are moved to a separate line according to the style manual

However, it's better to let the author decide if { should be on a separate line because

{ on a separate line isn't significantly more common than not having { on a separate line
both styles can easily be read
it's not a significant benefit that one (rather than both) style is used in a project (it's however a benefit that a function use the same style)
choice increase utility (i.o.w. if both styles are allowed total utility is increased)
flexible style reduce the discussion about code style which is beneficial because style is unimportant

[03:01] <_RachelB> there was also a trailing tab somewhere

This has been removed by detecting trailing whitespace with

git log -1 --check

Update: By removing them with https://github.com/mirror/dolphin-emu/issues/38#issuecomment-19506708

john-peterson commented 11 years ago

Trailing space

Problem

VS 10 and 11 when copying text

sometimes create unintentional trailing space
paste CRLF in a LF file (instead of converting the clipboard text to the file's LF before pasting it)

resulting in the need for f.e. these commits that remove them

8da425b "Also killed off trailing whitespace and dangling elses."
2316cb6 "Also killed off some trailing spaces/tabs."

Solution

Use this in .git/hooks/pre-commit because

that avoid trailing space (and subsequent removal) commits

#!/bin/bash

if git-rev-parse --verify HEAD >/dev/null 2>&1; then
    against=HEAD
else
    # initial commit: diff against an empty tree object
    against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi

# find files with trailing whitespace
for FILE in `exec git diff-index --check --cached $against -- | sed '/^[+-]/d' | sed -r 's/:[0-9]+:.*//' | uniq` ; do
    # remove trailing whitespace
    sed -i 's/[[:space:]]*$//' "$FILE"
    git add "$FILE"
done

# verify
if [[ `git diff --cached --check` ]]; then
    git diff --cached --check
    exit 1
fi

Use this alias because that make it easy to avoid ending space removal

git config --global alias.commitn "commit --no-verify"
git config --global alias.amendn "commit --amend -CHEAD -a --no-verify"

john-peterson commented 10 years ago

Commit unmodified commit or branch

Merge of "Fixing wiimote savestate and recording"

Why were the Wiimote state code removed from the commit? Answer: Because they were commited in a previous commit. However if I wrote them it's better than it's in a commit where I'm author

i didn't feel like going through your commits, and remaking them without all the other unrelated changes.

It's better that I change the commit by being told what to change f.e. with

a description of the changes (with an inline comment if it's beneficial)
a diff from the commit

instead of someone other than me because

it's beneficial to discuss every change in the commit with the author
it's important that the author approve the exact commit because it's inappropriate to commit a patch that the author doesn't agree with
this PR isn't auto-closed if the commit is modified, it will say "Closed with unmerged commits" instead of "Pull request successfully merged and closed"
the common opinion is that it's more efficient to explain the changes to the author instead of changing the commit

[02:59] <_RachelB> JPeterson: no [03:05] <_RachelB> and i see no real value in doing it

Why?

john-peterson commented 10 years ago

Close pull request

[02:49] @Sonicadvance1 JPeterson, Do you ever close your pull requests?

Yes because

there are closed PRs

[02:52] @Sonicadvance1 Because pull request #2, no. JPeterson

[02:54] @Sonicadvance1 JPeterson, Pull request #5, still no

The reason they aren't closed is

thay aren't merged

john-peterson commented 10 years ago

Standard

@Matt_P stop using commas for numbers ffs [02:49] @Matt_P it is unreadable when mixed with code [02:50] @Matt_P max(4,92, 4,88)

The comma is the default ISO decimal sign

There's no instruction to use another decimal sign than a comma if a comma also separate number groups because that would create uncertainty regarding which letter the decimal sign is

john-peterson commented 10 years ago

Squash now or later

[14:25] <_RachelB> JPeterson: not sure what your most recent change is

it's the code related to "wait" (search for "wait" in https://github.com/mirror/dolphin-emu/pull/9/files)

(why do you amend commits instead of making new ones?)

[14:38] <_RachelB> the incremental changes in this case are relevant. When they are not, you can post a patch with all of the changes. If you're just changing a variable name, a typo, etc, and they are obviously not relevant, then fine, amend the commit, but that's not the case here

The reason I've amended the commit is

the incremental changes in a pull request are sometimes of no value to identify later (f.e. an omission to define a variable)
there are benefits with having one logical change in one functional commit (rather than incremental commits) in the log because its changes can then be viewed from one commit rather than multiple, and if reverted it involves only one commit rather than multiple.
it concerns me that I might not be asked if I want to squash any patches before they're merged. If the commiter say that this will be asked this then this concern is reduced (the concern is reduced rather than removed because the commiter can forget to ask). Confirm that I'll be asked "do you want to commit the PR?" before it's committed because that give me an opportunity to squash the PR

I want to squash this PR before commit because

it's a low probability that the file system mtime code will be used (instead of the state header code or elsewhere)

This https://github.com/john-peterson/dolphin-emu/compare/_diff...state (doesn't work, it should be this https://gist.github.com/john-peterson/5481542) is the diff with the previous patch created from

# find previous patch
git log -g --grep=mtime
# copy master to _diff
git branch master _diff; git checkout _diff
# apply previous patch  
git cherry-pick b25dbca83d70b2aacb33622861f9382203767bfd
git add -u; git commit -m "diff"
# difference
git diff --stat -p _diff..state

john-peterson commented 10 years ago

GitHub notification

Problem

GitHub sent mail notification when an issue or pull request is updated to a user that's

a "participant" (has created a post in the issue)
quoted with "@username"

Discussion

[03:53] <_Sonicadvance1> JPeterson, wtf did I say about modifying history?

which argument is this a reply to?

[03:57] <_Sonicadvance1> all of whatever this nonsense is in my email box that I keep deleting

what message is mailed to you?

[03:58] <_Sonicadvance1> I don't know any longer, it is deleted

If you refer to a notification about an updated GitHub issue or pull request

the reason that you receive that is in the topic "Problem"

john-peterson commented 10 years ago

IRC

Paste posts in IRC

[14:36] <_RachelB> why would you respond to what i said in irc on github, …

I post a link to the message instead of paste the message in chat because

Markdown isn't parsed in chat which makes the message less efficient to read
the message sometimes reference other parts of its post by using f.e. the text "in the topic Topic" which content isn't clear in chat because it's not clear which post the pasted text is from
it's not a significant disadvantage to read my comments here rather than in IRC

… without so much as telling me you did so?

The reason for a delay between authoring a message and informing the recipient is

the arguments are unclear and might be more clear in the future

[15:35] <_RachelB> it is a huge disadvantage

Why?

[15:44] <_RachelB> JPeterson: because it's absurdly slow, confusing to have half the conversation in one place, and half in another, and cumbersome to go back and forth

Log difference

[14:37] <_RachelB> If you want a record of discussion, feel free to post logs

Saving the arguments is different from a log because

a log contain communication related to more than one topic (rather than arguments related to one topic)

Saving communication

The benefit with writing communication I participate in at GitHub is

that makes it easier to analyse the communication
- in the future
- by a person that didn't read the IRC communication
it's easily accessible because
- referencing text on GitHub is efficient because there's a direct link to the post (the anchor #issuecomment-)
- github.com pages load fast
- Markdown makes text easier to read
the message can be edited to improve clarity

[03:32] <_Jasper> [paraphrased] Don't save at GitHub what I write in IRC because

I want the option to approve which message I write on GitHub because

an argument in a chat rather than post interface might not be an accurate representation about my opinion because it might not be carefully considered

Do you allow saving your argument at GitHub if you receive the option to

change quotes from you want to change (f.e. because that makes your argument clearer)?

Describe the disadvantage with writing your arguments at GitHub because

it's not clear why it's a disadvantage
a message in a IRC channel not protected by password is public rather than non-public
why is it a significant disadvantage that communication I participiate in is saved on GitHub?
a commit should be based on logic rather than if communication about it is saved at Github
saving the the communication about the commit at GitHub doesn't have a negative effect on logical analysis

Your arguments (rather than only my arguments) are written at GitHub because

knowledge about all arguments (rather than not all arguments) in the communication makes it easier to analyse the communication

Is this the reason for the opinion that your interest in code is significantly lower if it's written by me rather than not written by me?

Why don't you want to analyse the code as you would analyse it if you didn't have knowledge about the author?

Compliance

To comply with this

I've changed your communication from a quote to a paraphrase in the topics

"Track grabbed state in X server rather than in client" in Adding capture cursor function
"Discussion" in Input configuration rounding
"Jasper" in Travis CI
"Saving communication", "Text organisation", "Efficient compromise" in GUI

because

a paraphrase compared to a quote

contain the substance of an opinion rather than also private information

keeping a paraphrase rather than removing also the paraphrase

it's significantly less efficient to analyse a communication if arguments from one or more participants aren't present

if there is a value in analysing the un-paraphrased text

I've saved the paraphrased comment in a private (rather than non-private) file for reference for a potential disagreement about the paraphrasing

Text organisation

[08:08] <_Jasper> [paraphrased] don't use bullet points in text written to me because

it invokes negative emotions related to structured rather than non-structured communication

However

I might write a a uniqe argument on a separate line (rather than more than one uniqie argument on the same line) because that makes it easier to

identify separate arguments
reply to every (rather than not every) argument

Punctuation preference shouldn't have a significant negative effect on meaningful communication because

punctuation preference should have a significant impact on efficient communication
the implied opinion (which is valuable because it's logical) in the discussion about punctuation (exclamation points) preference in Seinfeld (season 5 episode 4) (which has a negative effect on logical communication) is that punctuation preference should have a less significant impact than what is has in the narration

Negative emotions shouldn't have an effect on the code analysis because

the analysis should be based on logic
emotion doesn't have value for logical communication

Compliance

To comply with this I

when writing a message to you in a chat (rather than post) interface

not use the letter "*" or another bullet point punctuation as the first letter of a line
remove the letter "*" from the beginning of a line when copying an argument that use Markdown

Efficient compromise

[08:08] <_Jasper> [paraphrased]

consider all opinions carefully rather than not carefully

don't be inappropriately reluctant to make a logical change

because

it's inefficient to analyse a contested change inappropriately much

I agree with this argument

Multiple channels

[17:36] @neobrain JPeterson: take development related talk to #dolphin-dev, please

Why?

[17:38] @neobrain JPeterson: because this is just the off-topic chan and some people might not read stuff that is being written here at all

There's no benefit with having Dolphin related communication in more than one IRC channel

john-peterson commented 10 years ago

GitHub compared to Google Code

delroth: Cool, why is that on Github?

[10:33] <_neobrain> JPeterson: did I miss the part where you explained why you're only replying to people via github. Also, what's the reason of reason github instead of what EVERYONE else is using anyway?

The repo is mirrored to GitHub because compared to Google Code

it allow posts to be changed and removed that's useful because it allow making a post as clear as it can be

[20:49] <_delroth> neobrain_: I'm just here to kb JPeterson next time he posts a github link

Is there a script in #dolphin-emu that set "/mode #dolphin-emu +b JPeterson" if

the user JPeterson write a message that contain the text "github.com"?

Describe the benefit with not allowing the text "github.com" in #dolphin-emu or #dolphin-dev because

it's not clear that it has a net benefit

john-peterson commented 10 years ago

Notify recipient about message

[14:36] <_RachelB> … without so much as telling me you did so?

The reason for a delay between authoring a message and informing the recipient is

the arguments are unclear and might be more clear in the future, so that a delay is appropriate

[10:53] @neobrain JPeterson: it's not organized if no one knows that you replied to his question

I agree that it's appropriate that the author inform the recipient about a message

[22:06] @yolobrain JPeterson: you still haven't responded to my comment which criticized you for saying "I agree that it's appropriate that the author inform the recipient about a message" without actually telling me that you had responded to me via github

The reason that you aren't informed about this reply is

the topic "Notify recipient about message" doesn't have significant meaning because there's no difficulty in informing the recipient about a message

john-peterson commented 10 years ago

Remove mirror/dolphin-emu after its metadata is exported

[13:06] <_delroth> JPeterson: could you take down github.com/mirror/dolphin-emu now that github.com/dolphin-emu/dolphin exists?

mirror/dolphin-emu should be removed when there's a GitHub function to export its metadata (https://github.com/mirror/github/issues/2) because

a repo mirror with unexported metadata (issues) shouldn't be removed because it cause information loss

Read only mirror/dolphin-emu

mirror/dolphin-emu should be made read only when there's an option (https://github.com/mirror/github/issues/18) for that, because

it can prevent unintentional pull requests in mirror/dolphin-emu rather than dolphin-emu/dolphin

Prevent issue creation in mirror/dolphin-emu

The mirror/dolphin-emu

"Description" is changed from "https://code.google.com/p/dolphin-emu" to "Use dolphin-emu/dolphin instead of this repo"
"Website" is changed from "" to "https://github.com/dolphin-emu/dolphin"

because

it prevent accidental issue creation in mirror/dolphin-emu instead of in dolphin-emu/dolphin

mirror / dolphin-emu

Repo workflow #38

Code style

Trailing space

Problem

Solution

Commit unmodified commit or branch

Close pull request

Standard

Squash now or later

GitHub notification

Problem

Discussion

IRC

Paste posts in IRC

Log difference

Saving communication

Compliance

Text organisation

Compliance

Efficient compromise

Multiple channels

GitHub compared to Google Code

Notify recipient about message

Remove mirror/dolphin-emu after its metadata is exported

Read only mirror/dolphin-emu

Prevent issue creation in mirror/dolphin-emu