search

solid / authorization-panel

Github repository for the Solid Authorization Panel
MIT License
19 stars 20 forks source link

updates and improvements after aaron's fixes #275

Closed bblfish closed 2 years ago

bblfish commented 2 years ago

These are fixes I made after Aaron's corrections. We were working on them simultaneously so I had a a lot of work dealing with merge conflicts later (hence two PRs that were aborted).

Note: in the future could we have the scribe create short 80 or 70 char lines so that one can edit small chunks of text at a time. Here a small typo changes a whole paragraph, increasing the likelihood of merge conflicts.

TallTed commented 2 years ago

@bblfish - You marked at least this suggestion resolved, but it was neither applied nor answered.

This is doubly-problematic since this PR has already been merged.

Please don't do that!

I really did not expect this, after your lengthy argument with @csarven (who did not review this PR yet, though you had requested such, which I would have thought would lead you to wait for that review before you merged it) about the unannounced change in process!

I really don't want to spend double-time reviewing all my change requests one by one to check whether they've been applied, but I see no other option... Unless you will go through and un-resolve the changes which were not applied, and make comments explaining why on each, which can then be discussed before resolution.

bblfish commented 2 years ago

@bblfish - You marked at least this suggestion resolved, but it was neither applied nor answered.

Ah. Ok. That was

So I thought that was pretty safe.

This is doubly-problematic since this PR has already been merged.

Please don't do that!

I really did not expect this, after your lengthy argument with @csarven (who did not review this PR yet, though you had requested such, which I would have thought would lead you to wait for that review before you merged it) about the unannounced change in process!

you are right.

There is a bit of a problem with the PR method right after the call as a number of people sometimes contribute together. And PRs are less visibly about the content than editing the text as you do directly, which is more tedious though. I can't get some spell checking form IntelliJ to work that way.

I really don't want to spend double-time reviewing all my change requests one by one to check whether they've been applied, but I see no other option... Unless you will go through and un-resolve the changes which were not applied, and make comments explaining why on each, which can then be discussed before resolution.

TallTed commented 2 years ago

@bblfish - You marked at least this suggestion resolved, but it was neither applied nor answered.

Ah. Ok. That was

  • whether I was a scribe: I did not scribe
  • whether we had introductions: we did not

So I thought that was pretty safe.

sighs Github didn't do what I expected from that link, which would have been to show my suggestion. Instead, it brings you to the top of the very long page, with one resolved conversation at the very bottom (being the immediate change suggestion in question, one of MANY) which you must manually expand to see what I said about this line --

- * **Jusin B**: Interop doesn't necessarily require.. all these infra is in place.. but these are patterns where protocols have solved them.. have flows where ??? for average users over complex data.

-- on which I suggested several changes (there are five character sequences that differ between the above original and the below edited) --

+ * **Justin B**: Interop doesn't necessarily require all this infrastructure be in place, but these are patterns where protocols have solved them. Have flows where ??? for average users over complex data.

I also included a specific message to @justinwb, because this was the log of something he said --

@justinwb -- The last sentence needs your help to be complete, where the ??? is.

It appears there's no way to deal with this now, aside from manually reviewing each and every change suggestion I made on this PR, and submitting one or more PRs for those suggestions which were not applied, which makes me very unhappy indeed.

bblfish commented 2 years ago

@TallTed ah I see, very sorry.

@justinwb did you see Ted's comments at the bottom there https://github.com/solid/authorization-panel/pull/275/files#r728462020 ?