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

Fix 'PrintObj(1.)' #3395

Merged
merged 3 commits into from
Apr 10, 2019
Merged

Fix 'PrintObj(1.)' #3395

merged 3 commits into from
Apr 10, 2019

Conversation

DominikBernhardt
Copy link
Contributor

Fixes #3369 .

I received help for this PR from @sebasguts and @fingolfin

@DominikBernhardt DominikBernhardt added kind: bug Issues describing general bugs, and PRs fixing them gapdays2019-spring Issues and PRs that arose at https://www.gapdays.de/gapdays2019-spring labels Apr 8, 2019
Copy link
Member

@sebasguts sebasguts left a comment

Choose a reason for hiding this comment

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

I am fine with these changes.

However, deleting a library function is potentially dangerous. We should check if no distributed package uses it.

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.

Thank you! However, I have some remarks that ought to be addressed.

src/macfloat.c Outdated Show resolved Hide resolved
src/macfloat.c Outdated Show resolved Hide resolved
src/macfloat.c Outdated Show resolved Hide resolved
tst/testinstall/float.tst Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Apr 8, 2019

Coverage Status

Coverage decreased (-0.0009%) to 85.158% when pulling 0b2deb5 on DominikBernhardt:db/PrintObj into 27c845b on gap-system:master.

@fingolfin
Copy link
Member

For the record, I verified that no distributed package is using MACFLOAT_STRING_DOTTIFY

@DominikBernhardt DominikBernhardt changed the title Fix 'PrintObj(1.)' WIP: Fix 'PrintObj(1.)' Apr 8, 2019
@DominikBernhardt DominikBernhardt added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Apr 8, 2019
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.

Huh, actually meant to "request changes", not "approve"... Anyway, those changes apparently already are in the works, thanks

@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #3395 into master will decrease coverage by 0.01%.
The diff coverage is 75%.

@@            Coverage Diff             @@
##           master    #3395      +/-   ##
==========================================
- Coverage   85.16%   85.15%   -0.02%     
==========================================
  Files         697      697              
  Lines      344056   344050       -6     
==========================================
- Hits       293003   292960      -43     
- Misses      51053    51090      +37
Impacted Files Coverage Δ
lib/ieee754.g 100% <100%> (ø) ⬆️
src/macfloat.c 97.57% <50%> (+0.02%) ⬆️
lib/files.gi 76.44% <0%> (-6.95%) ⬇️
lib/files.gd 81.29% <0%> (-2.16%) ⬇️
lib/helpview.gi 47.57% <0%> (-1.43%) ⬇️
src/iostream.c 65.39% <0%> (-0.77%) ⬇️
lib/pager.gi 33.33% <0%> (-0.54%) ⬇️
lib/process.gi 43.5% <0%> (-0.5%) ⬇️
lib/init.g 83.11% <0%> (-0.37%) ⬇️
src/system.c 73.5% <0%> (-0.32%) ⬇️
... and 4 more

@DominikBernhardt DominikBernhardt removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Apr 9, 2019
@DominikBernhardt DominikBernhardt changed the title WIP: Fix 'PrintObj(1.)' Fix 'PrintObj(1.)' Apr 9, 2019
@wilfwilson wilfwilson added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Apr 9, 2019
src/macfloat.c Outdated Show resolved Hide resolved
src/macfloat.c Outdated Show resolved Hide resolved
src/macfloat.c Outdated Show resolved Hide resolved
@DominikBernhardt
Copy link
Contributor Author

@fingolfin I think I addressed all your comments.

@fingolfin fingolfin merged commit 359eee2 into gap-system:master Apr 10, 2019
@DominikBernhardt DominikBernhardt deleted the db/PrintObj branch April 14, 2019 19:04
@DominikBernhardt DominikBernhardt 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 Aug 21, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapdays2019-spring Issues and PRs that arose at https://www.gapdays.de/gapdays2019-spring 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.

PrintObj(1.) prints "1" instead of "1." or "1.0"
6 participants