fix: allowing reading items from preview feeds #166
Conversation
- Added another constructor for an in memory sqlite db for previews - Some small error handling cleanup - A bit of other conditionals to handle preview feeds
internal/store/store.go
Outdated
| } | ||
|
|
||
| func (sls *SQLiteStore) UpsertItem(item Item) error { | ||
| func (sls *SQLiteStore) UpsertItem(item Item) (int, error) { |
There was a problem hiding this comment.
I think the changes are much less invasive if instead of adding an additional return parameter you instead have this take a pointer-to-Item:
| func (sls *SQLiteStore) UpsertItem(item Item) (int, error) { | |
| func (sls *SQLiteStore) UpsertItem(item *Item) error { |
There was a problem hiding this comment.
Yeah that's a great idea.
cmd/nom/main.go
Outdated
| s, err := store.NewSQLiteStore(cfg.ConfigDir, cfg.Database) | ||
| var s store.Store | ||
| if cfg.IsPreviewMode() { | ||
| cfg.Ordering = constants.DescendingOrdering |
There was a problem hiding this comment.
Why does preview mode get a different default ordering? And in particular, one that overrides whatever value the user has explicitly configured?
There was a problem hiding this comment.
This was just because when you first open the preview mode it always opens in descending order. When you select an item then go back to the list, it would switch which seemed jarring. I can instead get the preview mode to use the users ordering.
There was a problem hiding this comment.
@guyfedwards should I keep the existing functionality or update this to use the user's default sort? I can go either way imo
There was a problem hiding this comment.
I would say use the users default sort 👍
Please add the
hackoberfest-acceptedlabel before merging :)