Drasil icon indicating copy to clipboard operation
Drasil copied to clipboard

Contributes to issue: Should add Well-Understood paper to front page description #3360 (Attempt)

Open daijingz opened this issue 11 months ago • 8 comments

@samm82 Wait a moment for the new updates. Inherited from previous PR #3686, but with some changes. Please wait for some further checks. Corresponding Issue: #3360

Newest inspection: Coding style requirements.

  • [x] Double spaces of indent levels.
  • [x] Camel case variable names
  • [x] Correct meaningful variable names.
  • [x] Single blank lines between top-level objects.

daijingz avatar Mar 26 '24 04:03 daijingz

Also @daijingz, it's better to make any changes in a branch on your fork so that you don't need to "revert" your changes on your master branch, effectively removing your work from the PR (like in https://github.com/JacquesCarette/Drasil/pull/3686).

samm82 avatar Mar 26 '24 18:03 samm82

Also @daijingz, it's better to make any changes in a branch on your fork so that you don't need to "revert" your changes on your master branch, effectively removing your work from the PR (like in #3686).

There are two existing branches in my personal machine for other issues, and I will use your described tools. @JacquesCarette sorry for concerns, I will improve in the future.

daijingz avatar Mar 27 '24 17:03 daijingz

Sorry everyone, there are some non-personal issues taking place on my PC, and I am going to go to repair stores tomorrow. I will get back tomorrow.

daijingz avatar Apr 09 '24 05:04 daijingz

@samm82 Newest update: have built two reusable wiki page sentences. This modification has some naming problems, and I am checking.

For the foldlList, I did not find some relevant code sections in the same program file. There is an example in this program file, however, it was used to represent an object list, where each hyperlink is separated by commas. However, this cannot work perfectly in the paragraph, as you can see, the input of foldlList has two input types: hyperlinks and sentences but this function can only have one type.

Can you show me the corresponding foldlList PR?

daijingz avatar Apr 17 '24 18:04 daijingz

Up to now my PR has a lot of commits, do not worry, later I will git rebase.

daijingz avatar Apr 17 '24 18:04 daijingz

For the foldlList, I did not find some relevant code sections in the same program file. There is an example in this program file, however, it was used to represent an object list, where each hyperlink is separated by commas. However, this cannot work perfectly in the paragraph, as you can see, the input of foldlList has two input types: hyperlinks and sentences but this function can only have one type.

Can you show me the corresponding foldlList PR?

@daijingz You should search the repo for "foldlList" to find how its used. If you can't find a PR/discussion about it (I think your git usage led to some of your work being overwritten), just search the codebase to see examples of how it's used.

samm82 avatar Apr 17 '24 19:04 samm82

For the foldlList, I did not find some relevant code sections in the same program file. There is an example in this program file, however, it was used to represent an object list, where each hyperlink is separated by commas. However, this cannot work perfectly in the paragraph, as you can see, the input of foldlList has two input types: hyperlinks and sentences but this function can only have one type. Can you show me the corresponding foldlList PR?

@daijingz You should search the repo for "foldlList" to find how its used. If you can't find a PR/discussion about it (I think your git usage led to some of your work being overwritten), just search the codebase to see examples of how it's used.

Before I notice there is an example on the second paragraph: QDU0O8_4K}@4HTBNF}~QXEY

For tonight's update, I use foldlList Comma List here, I guess this is the expected situation. @samm82 @JacquesCarette @balacij After reading some Wiki documentation, I have added some supplemental information for this PR. I realized the provided information's incompleteness.

daijingz avatar Apr 22 '24 04:04 daijingz

@samm82 Today I realized that webpage text inconsistencies may become a problem, so I inspected the existing text in the website to find any possible improvement places.

Scope: the whole About section. (Code is in About.hs) Possible improvement places: 3

  1. useage -> usage
  2. up to date -> up-to-date
  3. shows -> show

These are Grammarly suggestions. If you feel there is no need to change this, I can stay without any further changes, but I suggest modifying these words.

daijingz avatar May 15 '24 18:05 daijingz

Let me think about more possible improvements. (Updating)

daijingz avatar Jun 03 '24 04:06 daijingz

I would hold off on making any more changes in this PR (unless you learn how to use Git properly; it's hard to see what changes you've made since my last review, and just putting the issue number as the commit message doesn't provide any additional information about what change(s) was/were actually made!)

samm82 avatar Jun 03 '24 14:06 samm82

I would hold off on making any more changes in this PR (unless you learn how to use Git properly; it's hard to see what changes you've made since my last review, and just putting the issue number as the commit message doesn't provide any additional information about what change(s) was/were actually made!)

I am going to keep my updated version here (without any further changes). Before I just had continuous inspections on grammar, indentations, and content. I did not find a place worthy to change -> I feel, for some parts, it is better not to change instead of change. Up to now I did not find any violations with the Contributor's Guide.

daijingz avatar Jun 03 '24 16:06 daijingz

I am going to publish my new draft branch for another issue tomorrow. (Just to indicate my previous changes, all of you do not need to give any suggestions) (There is something I hesitate to determine.)

daijingz avatar Jun 03 '24 16:06 daijingz

To add to @samm82's comments about git, @daijingz, it would be nice if you could squash all of your commits into one or two commits. Also, you don't need to sync your branch as frequently as you do with master -- it's really only needed when there's a merge conflict showing up in the PR. Regarding your git commit messages, your objective with commits is to write a code change, ultimately. However, to make it understandable, you want to give readers of the code change (the commit) a quick summary (the commit message) before they read your code, so that they know what they're looking at and what to critique.

For example, in this PR, you are:

  1. Fixing a few typos in the existing website content
  2. Adding a link to the Well-Understood paper to the README.md
  3. Adding a few highlighted document links to the website content (namely, the original position paper, a poster, and the well-understood paper)

Now, these are 3 separate things you've done in this PR (which I think are all fair for one PR, and are all good, so thank you :) ). These 3 separate tasks can all fit into their own commits. However, I would make the commit messages just a tad more terse:

  1. Website: Fix a few typos
  2. README.md: Add a link to the Well-Understood paper
  3. Website: Highlight a few documents (original position paper, a poster, and the well-understood paper)

Of course, it would be expected that the commit messages sufficiently line up with what code was actually changed.

Actually, you can just make compress this down to just 2 commits:

  1. README.md: Add a link to the Well-Understood paper
  2. Website: Fix a few typos and highlight a few documents (original position paper, a poster, and the well-understood paper)

With these prospective commit messages, I think you can squash the existing commits (difficult) and/or soft reset the entire branch and recommit the changes gradually (simpler).

NOTE 1: You do not need to close this PR and start anew if you want to fix your commits. You will just need to use a force-push on your remote branch to fix the commit history.

NOTE 2: I don't think you should add any more changes to this PR. It looks fine as it is, in my opinion. The git commit history could probably be cleaned up a bit, however.

Aside: I probably have some auto commits around, but I fully intend to fix my own commit messages as well going forward.

balacij avatar Jun 05 '24 05:06 balacij

Sure, let me think about changing it (Really messy in the commit message right now)

Meet some problems right now, because the squash option is disabled. Maybe the failure is because of my branch Drasil_Copy_1 usages, so that all previous commits are temporarily disabled.

Up to now I am unable to get back to my master branch, and here is the error message: 6cd7bf250276f7b76a0ce069de93c243

I guess the most possible case is because Dr.Carette had not merged this PR from the master's branch, but merged another now from the copy branch. @samm82 Can you give me some help to fix this?

daijingz avatar Jun 05 '24 16:06 daijingz

What is that error message from?

balacij avatar Jun 06 '24 19:06 balacij

@balacij you provide some great advice above. Can you or one of the summer students add this advice to our Contributor's Guide? I like your advice about squashing commits and only merging with master when necessary. Your advice on commit messages is also valuable. We might already have some of this in the Contributor's guide.

smiths avatar Jun 06 '24 19:06 smiths

What is that error message from?

@balacij From Git Desktop, when I switched my current branch to master, it had this error message.

I tried to use commands today but it did not work. Also, when I selected multiple commits, by right-clicking, the option squash ... commits was in grey, meaning it was unavailable. I guess there are some problems, but I have not found an efficient way to solve this, and others feel unfamiliar with this error. Please help me.

@JacquesCarette There is a new PR completed today, should I submit before fixing this error? Here is the screenshot: 3d6ffbce94515e7d7e2f52fd98d42276

daijingz avatar Jun 09 '24 04:06 daijingz

@daijingz Can you please try out the branch in #3793 ? I think that should fix your issue.

balacij avatar Jun 11 '24 14:06 balacij

@daijingz Can you please try out the branch in #3793 ? I think that should fix your issue.

Sure

daijingz avatar Jun 11 '24 16:06 daijingz

Fixed. Now I can squash commits, and please wait a moment. There is a problem: on the GitHub desktop, I tried to drag my commits to the target squash commit. It shows a lot of options after choosing to squash. However, after determined to squash, I did not see any response from the commit message terminal. That is, the original two commits did not combine, and they stayed separate as before. @samm82 @balacij do you have office hour so that I can get assistance? But just wait for a few minutes, because I am still trying other ways.

f0676515414b748d9e41685551d20fd8

After this page, it shows that squash success, but I did not find any change on the same page (it seems like there is nothing change): e686895e7b931005582d78e9c695148c

daijingz avatar Jun 15 '24 03:06 daijingz

It looks like in c6971a3c2b610c68092eeca8306513f96ff58b47 - 767c18eb19b05cca308bee6acbe790b14812cd50, you duplicated my changes in #3793 -- I don't think you needed to do this. Instead, you could have pulled in my changes from my PRs source branch. Alternatively, now, you can merge in master or main (note: we're switching to main now, instead of master, as our 'main' development branch).

I'm not at all familiar with GitHub Desktop, and I don't normally host office hours during the summer. However, I can try to help you here. For starters, I think you should duplicate your working branch (i.e., create a backup before making commit history changes!). Then, you can do a git rebase -i HEAD~37 (using 37 because I see your PR has 37 commits, so rebase the last 37 commits) in your terminal to re-order and merge commits together (via marking some as being fixups of others).

balacij avatar Jun 20 '24 05:06 balacij

@balacij Do you have time tomorrow? Let me go to the campus if you are available, is it Ok?

Right now I feel some parts of the problems I have never seen before, and I did not have enough fixing information here.

daijingz avatar Jun 24 '24 15:06 daijingz

Regrettably, that time has past. Do you have any specific topics you need help with? I could try to direct you to some resources or help you here first.

balacij avatar Jun 27 '24 01:06 balacij

Fixing it, but right now I spend a lot of time working with squashing commits, with many different methods' usages. However, none of them have successful results.

Two used methods:

  1. Directly change from Github Desktop.
  2. Use commands on the cgywin64 terminal.

Both sides have the same result: the page just indefinitely processing, without any progresses.

More information is coming.

daijingz avatar Jul 02 '24 03:07 daijingz

I'm not quite sure what squashing has to do with this using GitHub Desktop app or Cygwin. However, if you're having troubles with checking out the repo still, it is probably worth it for you to switching to a 'Linux subsystem for Windows'-based development workflow. In other words, having all development go through WSL -- thereby allowing you to checkout the one file that broke compatibility on Windows temporarily.

Regarding squashing commits, you can either:

  1. git rebase -i and mark commits as 'fixup's of others to clean up the commit history (you can do this gradually! i.e., 2-3 rebases), or
  2. just reset the branch locally (i.e., without pushing! -- i.e., without closing the PR!) and manually re-apply the necessary patches, and then force-push to your branch, thereby erasing the old change history.

Going through (1) is surprisingly satisfying!

balacij avatar Jul 05 '24 17:07 balacij