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

Encode if we have readline in workspaces #2720

Merged
merged 1 commit into from
Aug 21, 2018

Conversation

ChrisJefferson
Copy link
Contributor

@ChrisJefferson ChrisJefferson commented Aug 21, 2018

Encode if readline is enabled in workspaces. Fixes #1079 and #1089

Also, will (hopefully) make it easier in future to add info to workspaces in a backwards compatible way (at least, backwards compatible that the errors will be reasonable when the workspace doesn't load).

@codecov
Copy link

codecov bot commented Aug 21, 2018

Codecov Report

Merging #2720 into master will increase coverage by <.01%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #2720      +/-   ##
==========================================
+ Coverage   75.75%   75.75%   +<.01%     
==========================================
  Files         478      478              
  Lines      241495   241501       +6     
==========================================
+ Hits       182938   182944       +6     
  Misses      58557    58557
Impacted Files Coverage Δ
src/saveload.c 68.25% <60%> (-0.03%) ⬇️
src/iostream.c 62.35% <0%> (-1.15%) ⬇️
src/hpc/threadapi.c 43.59% <0%> (+0.1%) ⬆️
src/stats.c 90.01% <0%> (+0.39%) ⬆️
hpcgap/lib/hpc/stdtasks.g 64.96% <0%> (+0.51%) ⬆️

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Change seems plausible over all, and fixes the issue.

But I'll not approve any PRs anymore w/o sensible labels; though I'll not complain if somebody else does and merges this PR (but then I hope they will also help us create the release notes for 4.10?)

src/saveload.c Outdated
{
static Char SyKernelDescription[256];
strcpy(SyKernelDescription, SyKernelVersion);
if(SyUseReadline)
Copy link
Member

Choose a reason for hiding this comment

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

clang-format? add issue labels?

Perhaps we can setup a bot to point this out automatically, it's a bit tiresome. Alternatively, we can also agree to not bother about clang-format anymore, and just let everybody format whichever way they feel like?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Id be happy with a bot. Maybe i can write a git script to run on my own machine that clangformats bevore each commit. (I actually do use formst on save on other projects i work on, but that reformats the whole file, so is no good for gap).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now reformatted

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 21, 2018
@fingolfin fingolfin merged commit 1f1d4e5 into gap-system:master Aug 21, 2018
@ChrisJefferson ChrisJefferson deleted the workspace-readline branch August 24, 2018 15:51
@fingolfin fingolfin added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants