Incorporated div-alg changes by Allen and Ángel

[codecov[bot]]

Codecov Report

Merging #56 into master will decrease coverage by 5.66%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
- Coverage   80.48%   74.81%   -5.67%     
==========================================
  Files          11       11              
  Lines        4647     5313     +666     
==========================================
+ Hits         3740     3975     +235     
- Misses        907     1338     +431     

Impacted Files	Coverage Δ
lib/div-alg.gi	67.09% <ø> (-18.81%)	:arrow_down:
lib/div-alg.gd	100.00% <100.00%> (ø)
lib/crossed.gi	73.91% <0.00%> (+0.14%)	:arrow_up:

[olexandr-konovalov]

@drallenherman @angeldelriomateos very good - so you see now that @drallenherman's commits to div-alg-bug branch in my fork resulted in updates of this PR. I will look now at the tests to see why they failed, and so see why there are conflicts between this branch and master after your commit.

[drallenherman]

Thanks Alex,

I have a few versions on my computer so the package info helps us to know which version we are testing when we are looking for mathematical bugs. It should just be a few more changes before we are satisfied and start polishing it for a new release. So expect not too many more ugly pushes like this one.

I downloaded Atom and have that working, seems nicer for managing the workflow on my end, and hopefully it will solve the end of line character issue.

Allen

[olexandr-konovalov]

Sounds good! Note however that Atom by default will also truncate spaces at the end of each line! So you may end up in a situation when a file has many lines changed, but actually most of them are whitespace changes. That may be a good thing to do consistently, but not good if it is mixed up with other changes. This article explains how to customise Atom settings: https://ourcodeworld.com/articles/read/514/how-to-remove-trailing-whitespace-on-save-in-atom-editor

Another good editor for Windows is Notepad++. Both Atom and Notepad have GAP support: see https://www.gap-system.org/Faq/faq.html#8.3

P.S. If replying to GitHub comments by email, please do not quote the text of the original email - it can be seen on GitHub anyway, so quoting it again makes the thread less readable. I have cleaned up your comment above and deleted it manually.

[angeldelriomateos]

Dear Allen and Alex,

I am still lost with the use of github.I fill like a old man unable to adapt to the new technology.

I am going to explain the situation at this moment because I fill this like too messy.

During our conversation by Alex set a fork called div-alg-bug where I think the latest version up to that moment was set.

Actually at that moment I had some improvements locally which I had not committed because I was doing some testing. During that testing I kept discovering some issues that I was communicating with Allen who made some small changes. However I didn't commit anything.

We should merge what I have locally at my computer and the small changes made by Allen.

Allen told me that he push a new fork of div-alg-bug, with version named -push56. However I cannot see it.

So I don't really know how I can include my improvements. How it is in my computer is really close to a satisfactory version but I don't know how to interact with github in a way that does not produce a bigger mess.

Sorry for that.

Ángel

[olexandr-konovalov]

@angeldelriomateos this is a normal collaborative scenario - you have made changes locally, and you can commit them, then pull changes make by others, rebase your changes (so they will look like they are made after changes by others, not before), and then push them so others can see and pull your changes. But we did not fully enact this workflow during our last conference call - probably two of us can get together on Monday in our timezone and try.

[drallenherman]

@drallenherman the commit 644d05c has a lot of whitespace changes which made impossible to understand where the actual changes are made. Could you please fix Atom settings ASAP to avoid this in the future, and let me know in which line numbers the actual changes were made? I will try to edit this branch and get rid of the unrelated changes then.

I changed my Atom whitespace setting to "Remove Trailing Whitespaces". I am thinking this is a one-time occurrence if I leave it like this, my future pushes won't have this problem. There is another setting called "Save without trailing whitespaces", is that better?

[olexandr-konovalov]

@drallenherman I think my comment could be misleading: so, https://ourcodeworld.com/articles/read/514/how-to-remove-trailing-whitespace-on-save-in-atom-editor shows how to navigate to the menu responsible for this behaviour, but what you have to do there is to UNCHECK "remove trailing whitespace" box to ensure that your edits only modify lines which you have actually modified.

[drallenherman]

I understand that Allen made some commits but has not pushed any file into div-alg-bug because I see no difference when I call git diff. Is that correct?

No there are two changes in div-alg: bugfix in GaloisRepsOfCharacters, improvements to GlobalCharacterDescent, and lots of new commands and examples in div-alg.xml. I am seeing the tests of new code in github that I finished this morning.

[angeldelriomateos]

I understand that Allen made some commits but has not pushed any file into div-alg-bug because I see no difference when I call git diff. Is that correct?

No there are two changes in div-alg: bugfix in GaloisRepsOfCharacters, improvements to GlobalCharacterDescent, and lots of new commands and examples in div-alg.xml. I am seeing the tests of new code in github that I finished this morning.

I understood yesterday in my conversation with Allen that if somebody push something into the common repository (div-alg-bug) then doing git diff, I would be able to see the differences and then I would pull the changes locally using git pull. However git diff does show up anything.

[olexandr-konovalov]

git diff is a command which displays difference between different versions. It can be used with various arguments. When it is used without arguments, then its default behaviour is to show the differences between the latest revision of the repository, and the current working version. If it returns nothing, it means that you do not have any uncommitted changes.

So, in your scenario, when you would like to see which changes you retrieved from the remote repository, you should rather call git show to the the latest commit, or git log to see the history.

Also, if you scroll down at https://swcarpentry.github.io/git-novice/reference.html you will see a dictionary which explains some terminology we are using - you may find that helpful to build a picture of the process.

[olexandr-konovalov]

This branch became quite convoluted - we spent quite a lot of time with @angeldelriomateos this morning trying to figure out why changes from ae59b026fd39dc3c8313c89fe11463dfba467403, and I restored them by cherry-picking that commit in ab0b811fb9a8b6fcf478dafc9719ccf8ef964df1.

This is the picture of this branch in SourceTree

and you can see a number of merges when you try to pull changes from alex-konovalov:div-alg-bug into your local clone.

One of the recommendations that will help to keep the history linear is to remember to do git pull (which will do the same as git pull alex-konovalov div-alg-bug) before you start any modifications, to ensure that you start on the latest version of the code.

Another one is to use --rebase option, as in git pull --rebase - in this case, Git will try to linearise your history and put your recent changes on top of the changes which it will pull from the remote repository. If those changes will not overlap, it should go seamlessly. If they will, you will have a conflict, which you will have to resolve as explained in https://swcarpentry.github.io/git-novice/09-conflict/index.html.

When this will be ready, I will merge this into master, and I will squash all changes in the branch into one cumulative commit. I suggest to do this as soon as possible, and start another pull request from a clean slate. There is now need to create a large pull request which contains several unrelated changes. For example, fixing a bug / adding new function / updating documentation for existing code may all be logically independent tasks suitable for individual pull requests. Let's do this ASAP, otherwise we are starting to lose time on non-productive version control forensics.

[olexandr-konovalov]

At least now, all tests pass (https://travis-ci.org/github/gap-packages/wedderga/pull_requests) and the code coverage for the changes is 100% (https://codecov.io/gh/gap-packages/wedderga/pull/56) so maybe a good time to merge it and start it with a clean slate?

[olexandr-konovalov]

Thanks @angeldelriomateos - will now pick up the fix and close this PR which will be now replaced by a new one - #58.

The changed line is not exercised by tests, as well as further code of that function - we should work on improving this.

[olexandr-konovalov]

This PR is closed - we will continue work in the new divalg branch in the main repository https://github.com/gap-packages/wedderga