Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

search: clean up feature flags#46086

Merged
limitedmage merged 7 commits into
mainfrom
jp/cleansearchfeatureflags
Jan 4, 2023
Merged

search: clean up feature flags#46086
limitedmage merged 7 commits into
mainfrom
jp/cleansearchfeatureflags

Conversation

@limitedmage

Copy link
Copy Markdown
Contributor

Remove experimental feature flags for notebooks, code monitoring, and search contexts as these features have been enabled by default for a long time now.

Test plan

  • Verify all three features still appear in the navbar and work as expected in other parts of the product (eg. search results action list).
  • Verify all three features do NOT show when running sg start oss.

@limitedmage limitedmage requested a review from a team January 3, 2023 18:49
@cla-bot cla-bot Bot added the cla-signed label Jan 3, 2023
@limitedmage limitedmage force-pushed the jp/cleansearchfeatureflags branch from 5b16763 to 2841a2e Compare January 3, 2023 18:50
@sourcegraph-bot

sourcegraph-bot commented Jan 3, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff f178717...d1f2781.

Notify File(s)
@fkling client/search-ui/src/results/StreamingSearchResultsList.tsx
client/web/src/search/SearchConsolePage.tsx
client/web/src/search/home/SearchPageInput.tsx
client/web/src/search/index.test.ts
client/web/src/search/index.tsx
client/web/src/search/input/SearchNavbarItem.tsx
client/web/src/search/results/StreamingSearchResults.story.tsx
client/web/src/search/results/StreamingSearchResults.test.tsx
client/web/src/search/results/StreamingSearchResults.tsx

@sourcegraph-bot

sourcegraph-bot commented Jan 3, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in OWNERS files for diff f178717...d1f2781.

No notifications.

@sg-e2e-regression-test-bob

sg-e2e-regression-test-bob commented Jan 3, 2023

Copy link
Copy Markdown

Bundle size report 📦

Initial size Total size Async size Modules
-0.01% (-0.40 kb) -0.01% (-1.18 kb) -0.01% (-0.78 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits d1f2781 and 24297ed or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@limitedmage limitedmage force-pushed the jp/cleansearchfeatureflags branch from 0d41cfa to 396944b Compare January 4, 2023 00:41

@valerybugakov valerybugakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@limitedmage limitedmage merged commit 449621c into main Jan 4, 2023
@limitedmage limitedmage deleted the jp/cleansearchfeatureflags branch January 4, 2023 17:45
fkling referenced this pull request Feb 21, 2023
This PR consolidates notebook routes into a single component.

I hope that doesn't interfere with the React router migration efforts.
It makes it easier to integrate notebooks with the SvelteKit app and is
also more in line with how other product areas are structured (e.g.
insights, code monitors, ...).

This also removes the use of `showSearchNotebooks`, which has been
removed in https://github.com/sourcegraph/sourcegraph/pull/46086.

(note: Adding people as reviewers who have better knowledge about react
router than me)

## Test plan

- When signed out I can see a list of public notebooks and open public
notebooks
  - Going to `/notebooks/new` redirects to the notebooks list page
- When signed in I see my own notebooks, can open my own notebooks and
can create new notebooks.
fkling added a commit that referenced this pull request Feb 22, 2023
…flags

These flags have been removed in #46086 and #46045 but were
accidentally(?) added back to the schema in (#46086).

Furthermore the `showSearchContext` flag was still accessed in a couple
of places, resulting in a broken experience if it was set to `false`
(see screenshot).

This PR removes the flags and removes all uses of `showSearchContext`
flag.
fkling added a commit that referenced this pull request Feb 22, 2023
…flags (#47956)

These flags have been removed in #46086 and #46045 but were
accidentally (merge conflict?) added back to the schema in #46086.

Furthermore the `showSearchContext` flag was still accessed in a couple
of places, resulting in a broken experience if it was set to `false`.


This PR removes the flags and removes all uses of `showSearchContext`
flag.

## Test plan

`grep`ed for the removed flags (`showSearchContext` is still used as
props in various places though).
Opened web app and verified that the context dropdown is properly
populated.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants