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

proofgeneral_HEAD: fix #25905

Merged
merged 1 commit into from
May 21, 2017
Merged

proofgeneral_HEAD: fix #25905

merged 1 commit into from
May 21, 2017

Conversation

Ptival
Copy link
Contributor

@Ptival Ptival commented May 18, 2017

Motivation for this change

As far as I can tell, the current derivation in master is broken, because of those changes:
ProofGeneral/PG@1a18e33
ProofGeneral/PG#177

It seems we don't need a patch anymore as a result.

@mention-bot
Copy link

@Ptival, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vbgl, @spacekitteh and @jwiegley to be potential reviewers.

@Ptival
Copy link
Contributor Author

Ptival commented May 18, 2017

@spacekitteh did you not run into this issue with
#25695
?

@joachifm joachifm merged commit 3ba63d0 into NixOS:master May 21, 2017
@gebner
Copy link
Member

gebner commented May 25, 2017

This PR broke the default proofgeneral package which still references the pg.patch file. I'm not familiar with proofgeneral versioning, should we make HEAD the default or revert the patch deletion?

@Ptival
Copy link
Contributor Author

Ptival commented May 25, 2017

@gebner woops, that was a bad oversight on my part!

I think we should probably keep the 4.4 around, in case HEAD ever breaks, it's good to have a more stable fallback no?

At least for now, let's revert the patch deletion. Should I:

  • revert this commit, then submit a new commit that does the other fixes w/o removing the patch
    or
  • just make a new commit to restore the patch
    ?

@gebner
Copy link
Member

gebner commented May 25, 2017

just make a new commit to restore the patch

I think that's the way to go here.

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.

None yet

4 participants