Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Refactor Narrow structure #1375

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[WIP] Refactor Narrow structure #1375

wants to merge 4 commits into from

Conversation

mounilKshah
Copy link
Collaborator

What does this PR do, and why?

This PR is to refactor narrow code.

Outstanding aspect(s)

  • How to change the entire narrow structure under Model class
  • How to modify tests as per the new narrow structure

External discussion & connections

  • Discussed in #zulip-terminal in Narrow refactoring
  • Fully fixes #
  • Partially fixes issue #
  • Builds upon previous unmerged work in PR #
  • Is a follow-up to work in PR #
  • Requires merge of PR #
  • Merge will enable work on #

How did you test this?

  • Manually - Behavioral changes
  • Manually - Visual changes
  • Adapting existing automated tests
  • Adding automated tests for new behavior (or missing tests)
  • Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

  • It is a minimal coherent idea
  • It has a commit summary following the documented style (title & body)
  • It has a commit summary describing the motivation and reasoning for the change
  • It individually passes linting and tests
  • It contains test additions for any new behavior
  • It flows clearly from a previous branch commit, and/or prepares for the next commit

@zulipbot zulipbot added the size: M [Automatic label added by zulipbot] label Apr 9, 2023
@mounilKshah
model: Add function to expose narrow to other files.
455ced7 
This commit contains get_narrow() and get_narrow_length()
which are used to provide data inside narrow and length of narrow
respectively to other components of the codebase.
@mounilKshah
helper: Replace direct access to Model.narrow with functions.
b49335f 
This commit replaces all instances where Model.narrow is directly
used with usage of functions Model.get_narrow() and
Model.get_narrow_length() in order to limit direct access to Model.narrow
to Model class.
@mounilKshah
messages: Replace direct access to Model.narrow with functions.
5c3228b 
This commit replaces all instances which directly access Model.narrow
with usage of functions Model.get_narrow() and Model.get_narrow_length()
to restrict access to narrow data structure to only Model class.
@mounilKshah
utils: Replace direct access to Model.narrow with functions.
2924a28 
This commit replaces all instances of direct usage of Model.narrow
with respective functions.
@mounilKshah mounilKshah added PR needs review PR requires feedback to proceed area: infrastructure Project infrastructure area: refactoring labels Apr 9, 2023
@zulipbot
Copy link
Member

zulipbot commented Apr 9, 2023

Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out!

@neiljp neiljp added further discussion required Discuss this on #zulip-terminal on chat.zulip.org and removed PR needs review PR requires feedback to proceed labels Apr 11, 2023
@zulipbot
Copy link
Member

Heads up @mounilKshah, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
area: infrastructure Project infrastructure area: refactoring further discussion required Discuss this on #zulip-terminal on chat.zulip.org has conflicts size: M [Automatic label added by zulipbot]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mounilKshah @zulipbot @neiljp