Relating to #2483 bookmark refactor, we’ve seen cases where bookmarks are:
1) losing prior ordering
2) new bookmarks add to bottom , not top of list (if intended, 👌, just curious if you intentionally wanted to reverse that)
3) lost bookmarks-perhaps mostly coinciding with private repos
Ok so I downloaded an older build off of TF and a few things of note:
Should we delete the local storage of bookmarks once they're migrated? One big nit I have with some apps is when they store files that can't be deleted without reinstalling the app and risk losing other things. I know the file is small in this case but yano?
I've just checked which ones disappeared and I think it looked more dramatic because I have an issue bookmarked which wraps onto 10 lines so takes up a lot of the screen and has since been deleted (Fully understand this one being gone)!
The main thing though is the removal of one of my private repos stored in a 2FA org.
On v1.26 this same repo is unavailable, it says 'There was an error loading the repository.' which is likely why it's not showing with the new live updating logic. However on v1.25 I can access this repo with no issues - wonder what change caused this as it doesn't seem related to bookmarks refactor? @rnystrom
It’s related. I moved the repo “has issues enabled” and “default branch” settings to a fetch in the repo VC. This API auth difference between 3 and 4 is really frustrating.
Sent with GitHawk
I'm honestly thinking about requiring PAT login as the only option.
Yeah that was my recommendation a month ago as well. With ability to reapply if desired and selective logout
Hmm yeah that's really frustrating!
I mean I'm completely up for just using PAT if it gives us more flexibility and less issues - but I do think we need to work out a nice way to explain this to users when shipping the update.
Wouldn't jump to my mind to switch to using PAT and don't want it to look like we've just logged users out or something!
Nope def wouldn't do that. Another option would be that when we get an OAuth error, show a more detailed alert on how to add a PAT to your account (requires updating account settings so you can override current login w/ a PAT tho).
Unfortunately I'm pretty burnt out on this project atm after this setback, API hiccups w/ the inbox redesign, and other things. It'll be a while before I get back to anything.
100% agree, it's these sorts of things outside of your control that send you completely crazy. The fact it feels like the V4 API is really a setback in some aspects versus the V3.
I do think your idea of showing a more detailed error with suggestions is definitely a great idea! Should we break this down into a separate issue for tracking and leave this for the reordering question?
Most helpful comment
I'm honestly thinking about requiring PAT login as the only option.