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.

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

Improve Steam install detection #1444

Closed
wants to merge 1 commit into from

Conversation

LarsOL
Copy link
Contributor

@LarsOL LarsOL commented Sep 5, 2015

If the steam game isn't found in the default spot. Parse the steam config file to get the path.

@pjf
Copy link
Member

pjf commented Sep 7, 2015

Oh cool! I always knew that in theory we could go parsing Steam's config files, but I didn't realise it could be this straightforward. :)

@LarsOL
Copy link
Contributor Author

LarsOL commented Sep 19, 2015

I am a bit curious (don't take this as criticism i am a new software engineer and just don't understand) but why do pull requests take so long to merge?

You seem to have a Continuous Integration system setup, and from my understanding merging often means dev's branches are less likely to conflict with each other, reducing the admin work.
i.e like this: #1434 which merged without conflicts when i put this pull request up, but now has conflicts.

If it's to do with stability, shouldn't there be a "Dev" branch that is continuous integration, with the "Dev" branch being merged to a release branch when it is deemed stable enough?

Thanks,
Lars

@pjf
Copy link
Member

pjf commented Sep 27, 2015

I am a bit curious (don't take this as criticism i am a new software engineer and just don't understand) but why do pull requests take so long to merge?

This is an excellent question. And the answer is nothing to do with stability, and all to do with human psychology! While everyone who has a collaboration bit has the ability to merge PRs, I think there's been a bit of a hesitancy to do so. I'm sure there are many reasons for this: merging a PR can make you feel responsible if you've missed any bugs in it, you may find reviewing code less fun than writing code, and the good old fashioned bystander effect where it's easy to believe that someone else will do it.

In our case, I suspect it's mostly bystander effect. I have scheduled sprints where I work on the CKAN, so it's easy to feel that for any given PR, I'll review and merge it if nobody else does. And that belief is indeed correct, except that we've built up a backlog of PRs as a result, especially in the last couple of months where I've been on the conference circuit and pouring hours into The Kerbal Book so we can hit our Christmas deadline.

I'd actually love for us to move towards something similar to what Rust do, where a bot assigns PRs to humans as soon as they come in.

@pjf
Copy link
Member

pjf commented Sep 27, 2015

Moving discussion to #1481, which is @LarsOL's excellent work with the results of code review on top (ie, a guard to make sure we don't choke on a bad config file). :)

@pjf pjf closed this Sep 27, 2015
@pjf pjf removed the pull request label Sep 27, 2015
pjf added a commit that referenced this pull request Oct 21, 2015
* 1444_adjust:
  Changelog joy for Steam location fixes (#1444, #1481)
  Added guards on Steam install detection (#1444)
  If the steam game isn't found in the default spot. Parse the steam config file to get the path.
pjf added a commit that referenced this pull request Dec 13, 2015
* master: (22 commits)
  Changed the AssignentSpacer of the FileIniDataParser instance to the empty string. This removes spaces around the '=' in the kde mimeapps and .desktop files and should enable KDE to parse the files without issues. Should fix #1498
  Fix typo bool->boolean
  Documet find_matches_files option
  Rebuild Ships Directory on Load
  Add extra property to have find match on files for backward compatability
  Actually default use_filename_version to false
  Remove TODO
  Automatically populate ci resource
  Fix spelling
  Update Changelog
  Support asset matching
  Make filename as version opt-in
  Allow find to match on files as well as directories
  Remove old Jenkins API
  More robust Jenkins support
  CONTRIBUTING: Grammar fix
  back to pattern
  Pattern -> enum
  Bring the schema up to date
  Changelog joy for Steam location fixes (#1444, #1481)
  ...
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