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.

Sign up for GitHub

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

Jump to bottom

Updated versions of various Jetbrains applications, including PHPStor… #22030

Merged
merged 1 commit into from Jan 24, 2017
Merged

Updated versions of various Jetbrains applications, including PHPStor… #22030

merged 1 commit into from Jan 24, 2017

Conversation

joncojonathan
Copy link
Member

Update versions for some Jetbrains products, including PHPStorm, IntelliJ and PyCharm (non-exhaustive list).

Motivation for this change

Version of PHPStorm and IntelliJ I installed was out of date - contributing a correction.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
    • Tested pycharm-community, phpstorm and idea-community
  • Fits CONTRIBUTING.md.

@Mic92
Copy link
Member

Mic92 commented Jan 22, 2017 •
edited

Hi, we usually have one commit per package update with the commit message:
packagename: <from_version> -> <to_version>.

@joncojonathan
Copy link
Member Author

Hi @Mic92 , Sorry! I'll remember that in future. Is that a problem in this case?

New to this, so expecting to learn as I go.

@Mic92
Copy link
Member

Mic92 commented Jan 22, 2017 •
edited

In case of intellij it might be acceptable to upgrade all in one commit as all components carry the same version. But please change the commit message to:

idea: 2016.3.2 -> 2016.3.3

globin
globin reviewed Jan 22, 2017
View reviewed changes
pkgs/applications/editors/idea/default.nix
@@ -340,12 +340,12 @@ in

datagrip = buildDataGrip rec {
name = "datagrip-${version}";
version = "2016.3.2";
version = "2016.3";
Copy link
Member

Choose a reason for hiding this comment

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

That looks like a downgrade, was that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't edit that part, not intentionally anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

@globin how do we fix the potential downgrade, do I need to perform a full recommit and make a new pull request?

Copy link
Member

Choose a reason for hiding this comment

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

no. leave this pull request open. You can added changes to your existing commit using git commit --amend and a git push --force origin update-jetbrains

Copy link
Member Author

Choose a reason for hiding this comment

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

@globin @Mic92 I've updated the commit message and corrected the datagrip version. I've also corrected the hash for it as that seems to have regressed.

I've tested datagrip too.

Copy link
Member

Choose a reason for hiding this comment

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

You don't seem to have pushed the fix to the downgrade, might need to git add before amending the commit

@joncojonathan
Copy link
Member Author

@Mic92 versions now specified in the commit message.

@joncojonathan
Updated versions of various Jetbrains applications, including PHPStor…
654167e 
…m, IntelliJ, PyCharm.

clion                 2016.3   -> 2016.3.2
datagrip	      2016.3.2 -> 2016.3.2
idea-community        2016.3.2 -> 2016.3.3
idea-ultimate         2016.3.2 -> 2016.3.3
pycharm-community     2016.3   -> 2016.3.2
pycharm-professional  2016.3   -> 2016.3.2
phpstorm              2016.3   -> 2016.3.2
ruby-mine              2016.2.5 -> 2016.3.1
webstorm              2016.3.1 -> 2016.3.2
@joncojonathan
Copy link
Member Author

@Mic92 @globin I think that's everything ironed out now. I've also added the remaining jetbrains packages.

Please let me know if you require anything further?

@grahamc
Copy link
Member

grahamc commented Jan 24, 2017

This looks fine to me, but do note that the commit message is definitely deviant from the norm. In the future, please upgrade one package per commit and format the first line of the message "foo: 0.0.0 -> 0.0.1"

@grahamc
Copy link
Member

grahamc commented Jan 24, 2017

Oh, I meant to say, thank you so much for the patches :) We definitely appreciate them!

@grahamc grahamc merged commit ea9f5ce into NixOS:master Jan 24, 2017
@joncojonathan
Copy link
Member Author

@grahamc No problem, and I've noted the commit message style for future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@globin globin globin left review comments

Assignees
No one assigned
Labels
12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@joncojonathan @Mic92 @grahamc @globin @Janik-Haag