Skip to content

Instantly share code, notes, and snippets.

@Niloth-p
Last active August 26, 2024 14:48
Show Gist options
  • Select an option

  • Save Niloth-p/db22a98962b204f6e4f67f4c5d98c7cd to your computer and use it in GitHub Desktop.

Select an option

Save Niloth-p/db22a98962b204f6e4f67f4c5d98c7cd to your computer and use it in GitHub Desktop.
Final Submission for GSoC 2024 with Zulip

image

Google Summer of Code 2024 with Zulip - Work Product

Hi πŸ‘‹, Niloth here. This is a summary of the work I did for Zulip and my personal experiences as a part of the GSoC 2024.

About Zulip

Zulip is an open-source team chat with topic-based threading.

Zulip combines the immediacy of real-time chat with an email threading model. With Zulip, you can catch up on important conversations while ignoring irrelevant ones.

Why Zulip? - Efficient communication with organized team chat.

🌐 Github Links

Repositories

PRs worked on during GSoC

Commits merged during GSoC

Code Reviews

🌟 Contributions

From the moment I encountered Zulip, I've found myself irresistibly drawn to working with Zulip. I’ve been fully immersed since late March. Now that GSoC has ended, it’s hard to move on and not spend all of my time with Zulip. It’s been an incredibly rewarding experience, and leaving feels emotional.

Below are my contributions during the community bonding and coding period of GSoC.

⌨️ Hotkey Bindings and Help Entries

πŸ—‚οΈ Re-categorization of hotkeys and Improving help texts

Hotkeys are central to the user experience in a TUI app, making their corresponding help entries equally critical.

Help entries are integral to

  • the in-app Help Menu,
  • the random help hints in the footer,
  • and the help documentation.

Creating new help categories directly improved the Help Menu significantly, and it also laid the groundwork for enhancing the hotkeys' user experience across the app. This work was essential for implementing the help context feature.

Arriving at optimal groupings of help entries required careful consideration.
And tweaking the help descriptions required an even more nuanced approach, to prevent layout issues while also clearly conveying their purpose within their categories.


#1496 Standardize hotkey help text

  • 7edecc Improve case consistency across hotkey help texts.
  • 379585 Standardize choice of words in hotkey help texts.

#1494 Improve the handling of editor actions as a help category

  • b8c0ab Add missing key combinations provided by urwid-readline, as new hotkey commands.
  • d3dd70 Create two new help categories for editor hotkeys.
  • 49b67c Re-order editor text manipulation key bindings by importance.
  • cd34d5 Improve descriptions of editor hotkeys.

Previously, the urwid-readline editor was utilized solely in the compose box, limiting GNU Readline hotkeys to that context. However, after the implementation of #1492, all editors have been upgraded to use the urwid-readline editor, making the hotkeys available across all editors.

To reflect this change, we update the help menu entries, by moving all the urwid-readline help entries out of the "Composing" category and into their own categories "Editor: Navigation" and "Editor: Text Manipulation".


#1520 Create new help categories related to opening compose box

  • 9e7de8 Add a new help category for opening compose box.
  • 5b6f02 Rename the help category of message compose box actions, to emphasize the difference between hotkeys that open the compose box, and hotkeys that are applicable from inside the compose box.

#1516 Group message view switching commands and improve descriptions

  • fa63fa Add a new help category to group message view switching commands.
  • fbaa17 Re-order the 'narrowing' key bindings.
  • 70032a Improve descriptions of hotkeys that switch narrows.

#1515 Add new help categories to remove parentheses (context) from help texts

  • aa50cb Add new help category for stream information popup.
  • 8ef0cd Add new help category for message information popup.
  • 22ced5 Add new help category and command for user list actions.

Some hotkeys required mentioning the context in parentheses, due to being aligned on the functionality axis. This PR removes that requirement, by re-categorizing such help entries into a context-based axis of help categories.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ“ƒ Separate help entries for multipurpose hotkeys

#1497 Segment the generalized ENTER command

  • 65e566 Break down the generalized ENTER command into more specific commands.
  • f423e9 Add space key as an additional trigger of ACTIVATE_BUTTON command.

#1514 Decompose the GO_BACK command

  • 6823e5 Extract CLEAR_SEARCH command from GO_BACK command.
  • 03cb3d Extract EXIT_POPUP command from GO_BACK command.
  • 97e8e2 Convert remaining GO_BACK command use into EXIT_COMPOSE.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ†™ Upgrading the key bindings map

These PRs were deferred to prioritize the implementation of help contexts, with plans to revisit them afterward.

I proposed enhancements to the KeyBinding TypedDict, enabling key bindings to be associated with multiple categories and contexts, each with its own distinct hotkey description. This was proposed in anticipation of several upcoming features such as:

  • Allowing key bindings to belong to multiple help menu categories ("multi-categories").
  • Introducing dynamic help texts that adapt to different contexts and states.
  • Introducing a more flexible context system by transitioning to multiple contexts instead of the discussed parent/child context hierarchies.

#1519 Add multi-category support for key bindings1

Prior to proposing the more extensive updates to KeyBinding for supporting multiple contexts and dynamic help texts, I implemented multi-category support for key bindings. This PR made it possible for a hotkey to be categorized under several help menu categories, enhancing clarity. By just adding a field to the KeyBinding TypedDict instead of modifying it. The same approach can be extended to support multi-contexts as well, but would not accommodate dynamic help texts.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

βœ… Key Binding Validations

#1498 External hotkey commands: tagging, linting and checking sync1

  • 84f501 Add common suffixes for all the urwid-readline hotkeys.
  • 514db2 Add linting for external command usage, using regex.
  • fa9251 Check sync of readline shortcuts with urwid-readline's keymap.
  • 0495e2 Add suffix & linting to general terminal commands as well.

Originally based off of a single feedback comment, other more robust approaches were considered later. This work was put on the back burner, due to its lower priority as well as the approach not having solidified.

#1518 Lint the validity of help category values

This is a minor developer feature that I proposed to address issues where mistyped categories could lead to silently disappearing help entries. This has now been absorbed into the linting refactors and updates done in #1484.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

ℹ️ #1484 Add feature: Help Context1

Insights

View insights
  • Reiterating the importance of hotkeys and help in the Terminal client, this new feature aims to improve the UX.

    Currently, the app displays a random help hint in the footer. A new hint is generated only when a footer event is triggered and it ends returning back to the random hint.

    Our goal is to use the focused context to generate only relevant help hints. And to provide users a help menu that shows all the hotkeys they can use from that context.

  • My previous implementation involved localized state update calls to the "context" property, across the codebase. While the feature worked as expected, Neil Pilgrim pointed out that a better implementation would keep all the context management code centralized.

    Consequently, I discarded 14 of the 22 commits from the initial implementation (view the branch with the alternative localized version), including all the tests added, and started afresh. While it was beneficial in getting me exposed to the different parts of the codebase, and it was also my first time writing new pytests, this was a valuable lesson in pacing myself, and the risks of advancing too quickly without taking the time to discuss more.

  • Neil Pilgrim taught me the pros of picking more issues to branch myself out, instead of pushing forward a single issue till completion, in order to allow more time for discussions and feedback.

  • The revised implementation leverages Urwid’s main loop to track the focused widget and map it to recognizable contexts. While doing this mapping work, I also looked into the Widget Hierarchy that I had proposed earlier. And this time I delayed writing the tests, from my last experience. But also because testing is an absolute challenge for this implementation, due to it not fitting into any of the existing patterns in the test base.

  • During our weekly call, I brought up whether to use a single PR or multiple stacked PRs for this feature. While I initially leaned toward stacked PRs for simplicity, we concluded that a single PR would be better given their dependencies, and that there's nothing wrong with having a PR with 22 commits.

  • While the logic was ready, the actual context values themselves were dependent on the re-categorization work to be completed. Thus, this work remained blocked for two months.

  • In the interim, I refactored some code to simplify context handling, which led to a substantial increase in commits, bringing it to a total of 34 commits.

  • While I made sure that the commits themselves are simple, well-documented and that the PR is easy to navigate, I learned only recently that the 34 commits might have discouraged timely reviews.

Commits

View commits

Footer updates and refactoring

  • de8d7a Make the Help hotkey suggestion in the footer more conventional.
  • 9fccce3 refactor: ui/core/boxes: Added function reset_footer_text() to reduce overloading of set_footer_text().
  • 13410a refactor: ui/core: Separate footer events from set_footer_text().
  • b9b5ee Add state variable to track live footer events.

Rewrite Lint-hotkeys: General Improvements

  • af00b4 Handle the doc file does not exist exception, and add a separate error message.
  • d2e0c1 Remove the write_hotkeys_file() function.
  • 4e8783 Make int error flag boolean & use SystemExit.
  • e95487 Rewrite function docstrings and comments.
  • 4ed23c Rename get_hotkeys_file_string().
  • 971777 Rename ambiguous keyword "action" to "batch".
  • 11a7ea Improve variable naming of generated dict.
  • 323725 Refactor the flow to increase efficiency. Add a constant Dict to call read_help_categories() only once now.
  • 188bd7 Delay generation of file string when linting.
  • 0dd8aa Split out the help text linting function.
  • 528648 Split out the function that lints categories, to allow linting all the groups before handling the errors.
  • 4e7a4f Restructure the error messages of lint_help_text() to reduce redundancy when there are several linting errors, and improve formatting.
  • efc8f82 Lint for typos in key_category values.

Refactor Lint-hotkeys: in preparation for linting contexts

  • ac8e71 Use help_group instead of HELP_CATEGORIES. Part of adding generic variables in preparation for contexts.
  • 58ec33 Create generic variable entries_by_group. Part of adding generic variables in preparation for contexts.
  • 3ce52d Create generic variables and types for key_category. Part of adding generic variables in preparation for contexts.
  • 491168 Replace 'category(ies)' with 'group(s)'. Part of adding generic variables in preparation for contexts.
  • b35e0c Delete constant OUTPUT_FILE_NAME, compute it. Part of adding generic variables in preparation for contexts.
  • 001814 Replace usage of OUTPUT_FILE. Part of adding generic variables in preparation for contexts.
  • e2c7e5 Generalize the traversal of key group values.
  • f908dc6 Use bundled group values.

Adding contexts + linting

  • f5d0bc4 Add a contexts field to Key Binding, and a dict of supported hotkey contexts.
  • a8dd4d Add argument to generate a keys file grouped by contexts.

Use contexts in footer hint

  • a76da7 Use the help contexts in generating random help tips.
  • 9c48eb Include previously excluded random help hints.
  • 2e1b0f Enable footer texts to display contextual help tips, by adding a context property and updating footer text on context changes, if a footer event is not already in progress.
  • 1b6ad6 Display the context of the footer hint.

Track context switching

  • f75a30 contexts/core/ui: Track the currently focused widget in the UI.

Contextual Help Menu

  • 58b1a9 core/ui/views/keys: Add a Contextual Help Menu.

Parent Contexts

  • 3513d7 keys/views: Add a mapping of contexts to "parent contexts" (broader contexts that they belong to).

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

βš™οΈ #1503 New Account Configuration Management System1

Feature Design

View feature overview

Supported config locations

  • Current default: ~/zuliprc
  • New directory: ~/.config/zulip-terminal/<account-alias>/zuliprc

To open ZT, use command zulip-term <account-alias>. Account = (Realm, Profile)

Configuration Folder Hierarchy: Creation

This is how the zuliprc files will now be created:

.config/zulip-terminal/
    account1/zuliprc
    account2/zuliprc
    account3/zuliprc

The profiles are made as directories, to be able to expand easily in future. For example, there have been discussions on breaking down the sections of the zuliprc file into separate files - the api file, the zterm settings (local and global custom configs), and the custom key bindings when they are introduced.

Commands

Account Creation:

zulip-term --new-account or zulip-term -n > Enter account alias OR zulip-term when there are no accounts found > Enter account alias

App Startup:

zulip-term - works if there is exactly 1 account found. If several accounts are found, error msg suggests using either zulip-term <account_alias> or the --config option. If no accounts are found, directly goes to creation.

Follow-up: In the error message for when several accounts are found, list all the account aliases (and their other details) and the current default path (if account found), along with the suggested commands. Later, this can be extended into a "pick the index" approach, for the config files located in the realm folders and the --config for others.

zulip-term <account_alias> - if no matching alias found, list all valid aliases (and their details) Follow-up: Support a "pick the index" approach.


Auxiliary Commands:

Listing

zulip-term --list-accounts - lists valid account aliases, their server URLs and emails, formatted as a table.

Follow-up: zulip-term --list-realms to list the realms that accounts exist for (may be useful to reduce the clutter when one has several accounts per realm).

Importing

zulip-term --import path/to/file > account alias Copies and moves the file into its appropriate position in the hierarchy.

Switching aliases

zulip-term --change-org-alias <old_alias> <new_alias>

The old alias is identified, the new alias is validated, and the index is updated. Output a confirmation message with the new alias with the user's URL/email.


Index:

This subfeature has not yet been implemented.

Index file path: .config/zulip-terminal/config-index.json

Fields tracked:

  • The time the index was last updated.
  • The number of valid accounts found inside the .config/zulip-terminal directory
  • Accounts - uniquely identified by their alias
    • An is_valid attribute that allows tracking invalid configuration files in the index
    • server URL and email

Sample file:

{
  "last_updated": "2024-08-06T14:52:00Z",
  "valid_accounts": 2,
  "accounts": [
		{
			"account_alias": "czo-me",
			"server_url": "https://chat.zulip.org/",
			"email": "abc@gmail.com",
			"is_valid": True
		},
		{
			"account_alias": "czo-botname",
			"server_url": "https://chat.zulip.org/",
			"email": "bot@zulip.com",
			"is_valid": False
		},
		{
			"account_alias": "recurse-me",
			"server_url": "https://recurse-center.zulipchat.com/",
			"email": "def@gmail.com",
			"is_valid": True
		}
	]
}

Before any ZT command is executed, the .config/zulip-terminal directory can be traversed to check if any files have been updated since the value in the last_updated field of the index, or if any of the listed accounts' zuliprc have been deleted, and if so, the mapping can be updated. This is to account for when users may accidentally manually change their file names, without using the given commands.

Commits

View commits
  • 59828e4 Rename fetch_zuliprc() to login_and_save(), and add docstring.

  • 8d17aa9 Split parse_zuliprc() to create a separate function resolve_to_valid_path() to return a valid zuliprc path.

  • ebd394f Set zuliprc_path via login_and_save().

    Previously, zuliprc_path was set directly in main().
    Now, we are setting it 2 levels deeper, to allow adding support for multiple zuliprc paths later, where we will dynamically compute the path, after taking user input.

  • 1bde5fd Use a constant for the default zuliprc path.

  • c0a0726 Add a function that gets the path to XDG_CONFIG_HOME.

  • d866880 Use the new config directory as a default zuliprc path, in addition to the previous default path "~/zuliprc".

    It is ensured that users who have only a single account can skip using the account alias argument that will be added.

  • a4038b2 Create account-wise zuliprc files in the new config path.

    This moves default zuliprc creation into the user's config directory.
    The --config-file option will no longer be able to save zuliprc files in custom locations.
    The new config path including the .config directory are handled safely, to allow creation even if they do not already exist.

  • ffdd5ee Add new positional argument account-alias to load config.

    The usage of the account-alias positional argument is made mutually exclusive with the config-file option.

  • 330d6c9 Add new argument --list-accounts.

    Lists all the accounts in the config path, by checking the directory structure. The home path zuliprc is not considered in this list.

  • 27473c1 List all accounts when the user enters an invalid account alias.

  • 817b46a Stop passing zuliprc_path to login_and_save().

  • 9681331 Add new argument --new--account to create a new account.

    • If --new-account is used along with --config-file,
      • If the given path is valid, that zuliprc will be loaded
      • Else, a new account will be created, but not at the given path.
    • If --new-account is used along with account-alias, the new account argument will be ignored.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸš€ More Help Menu / Hint Improvements

#1527 Add title-to-title scroll feature to Help Menu1

This was early work I had done, which I pushed as a PR much later, since there wasn't an issue for this - it was an idea inspired from discussions several years past. Even the hotkeys hadn't been finalized, and we are now exploring ways to generalize and extend this feature to the side panels as well.

  • 1b0d37 Add 2 new hotkey commands to refocus to view the previous/next help category section.

#1517 Add manual cycling of random hints in footer

This feature was added as it nicely complements the help contexts feature.

  • 0a11a5 Add hotkey command to generate new random footer hint.

Add periodic cycling of random hints in footer1

This was originally part of the above Footer cycling PR, when I had implemented both manual and periodic cycling while waiting for specs.

In this implementation (view branch), periodic cycling starts only after a manual trigger, requiring major refactoring for app launch integration. Since manual cycling was deemed sufficient, I removed the periodic cycling commits.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ› οΈ Bug Fixes

#1480 Handle new/empty direct message narrows better

I submitted this issue, but it was not formulated to contain a single well-defined issue. It addressed two distinct problems, with one of them being a subset of a more extensive, persistent issue. Thus, it was closed. However, it prompted the creation of new, comprehensive issues:

  • #1506 Improve handling of narrows (views) with no messages
  • #1505 'All messages' are not consistently accessible compared to P/#/f

In gratitude to the creation of these issues by Neil Pilgrim, I submitted PRs to both of these, mentioned below.


#1543 Handling Empty Narrows1

This PR resolves a long-standing collection of bugs.

Bug fixes and improvements in related code:

View bug fixes and improvements
  • Fixes UI issues in empty message views, listed in #1506

    • Empty streams #259, #607
    • Empty DMs (Separate handling for this specific case is planned but not yet implemented. Currently, it uses the default dummy message solution.) #1480
    • Empty searches (required separate handling) #580
    • Empty starred, mentions, all DMs - thread
  • Crash for user with no subscriptions #895

    Interestingly, this issue wouldn't have been identified as connected to #1506, had I not encountered a subset of it during my personal exploration with bots.

  • Improved related UI: Fix the recipient bar displaying "DONT HIDE"

  • Improved related code: Remove error handling for unreachable case, and simplify the flow.

The issue was mainly caused by the tight coupling of UI elements with the focused message. I explored several solutions, before settling on an approach using a dummy message.

Commits:

View commits
  • 4992db boxes: On opening application, display an empty recipient bar.
  • 936cf2 core: Track if the current narrow is empty.
    • Ideas inspired by the work done in #278 by Sumanth V Rao
  • 6cac57 messages/views: Disable keypresses from dummy messages.
  • f8503f messages: Customize recipient bar text for dummy messages.
  • dcb213 messages: Turn off recipient header for dummy messages.
  • 0fb5bf model: Replace the dummy message on receiving a new message.
    • Ideas inspired by the work done in #278 by Sumanth V Rao
  • df2feb messages: Remove error handling for unreachable case, and simplify the flow.
  • 299a1c refactor: messages: Allow datetime field of current message to be empty.
  • 344455 utils/messages: Add a dummy message for empty narrows.
    • Ideas inspired by the work done in #278 by Sumanth V Rao
  • 2a6629 core/boxes: Flash footer message on search misses, without narrowing.

#1512 Make the ALL_MESSAGES command work globally1

The keys a and Esc trigger the ALL_MESSAGES command. Since the Esc key serves multiple purposes, the ALL_MESSAGES hotkey could only be used when a message was in focus, unlike all the other "view feed" hotkeys.

This problem was intriguing and satisfying to solve, as it presented several edge cases that aligned perfectly with a clean solution once it was crafted.

  • 0bf31b refactor: ui_tools/helper: Add new enum Search Status.
  • 4f15ca views/boxes: Reset search only after verifying search status.
  • 2525a7 views: Make the ALL_MESSAGES command work globally.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ”„ Refactoring

#1537 Group message data1

Several new features by buddy contributor Sashank Ravipati required adding new fields to the message metadata. Previously, message data needed to be passed around several code and test files. This made the PRs for the new features too bulky with about 75 redundant occurrences, which was further exacerbated by formatting requirements that required the breaking of 1-line statements into 4-5-liners. Since I had some bandwidth, I refactored the message metadata into a single dataclass, to aid my buddy by simplifying the work needed for their PRs. View this initial branch here.

This was later improved to a newer version that avoids passing the data entirely, instead of a refactor. Currently, popups pass all data from parent to child to recreate the parent popup on exit. This gave me the idea for stacking popups, by maintaining a stack of Popup Views instead of destroying and recreating them, I eliminated the need to pass data.

  • 15a7c8 Use self to re-use instance variables in MsgInfoView class and the TestMsgInfoView class.
  • c734d2 Stack inner popups, add support for exiting all popups with a hotkey.
  • e31632 Delete all message info popup parameters from inner popups.
  • 9e5036 Delete the calling arguments from MsgInfoView's show_* calls.
  • 9051d4 refactor: views/helper/core/messages: Group message info in a TypedDict.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ“ Updating Integration Docs

The integration docs needed to be updated to adopt the new structure. This involved making section formatting changes, editing some instructions to adhere to a uniform writing style, replacing macros, and verifying some of the content.
It was a fun team experience working with Pieter CK, Adnan Husain, guided by Lauryn Menard, Tim Abbott and Alya Abbott.

List of all the merged commits

List of PRs and commits

#30733 Updating Github, Github Sponsors, UptimeRobot, Updown docs

  • c300b1c integration-docs: Update Github doc.
  • f29c270 integration-docs: Update Github Sponsors doc.
  • 4d6f50f integration-docs: Update UptimeRobot doc.
  • 74ecb3a integration-docs: Update Updown doc.

#30790 Updating Zapier, Zendesk docs

#30792 Updating Zabbix doc

  • 483b8c4 integration-docs: Update Zabbix doc.

#30793 Updating Wekan, Wordpress docs

#30794 Updating Taiga, Thinkst, Transifex, Travis CI docs

  • 4750c82 integration-docs: Update Taiga doc.
  • 5838025 integration-docs: Update Thinkst doc.
  • 46b5596 integration-docs: Update Transifex doc.
  • 3274128 integration-docs: Update Travis CI doc.

#30798 Updating Stripe, Statuspage, Splunk, Sonarr, Sonarqube, Slack-incoming docs

  • 1cd6d22 integration-docs: Update Stripe doc.
  • 7e4a1d6 integration-docs: Update Statuspage doc.
  • f3ba341 integration-docs: Update Splunk doc.
  • 1652d51 integration-docs: Update Sonarr doc.
  • f6f5ed5 integration-docs: Update Sonarqube doc.
  • 994af55 integration-docs: Update Slack incoming-webhook doc.

#30799 Updating TeamCity, Trello docs

  • eee2cb8 integration-docs: Update TeamCity doc.
  • b4b76cb integration-docs: Update Trello doc.

#30974 Fixes in updated integration docs

  • 525f1e2 integration-docs: Fix typo in Splunk doc hint.
  • ed885ec integration-docs: Format keyboard shortcuts in Trello doc.
  • cbe2677 integration-docs: Include suggestion for HTTPS hint in Stripe.

#31011 Updating Radarr, Raygun, Review Board, RhodeCode, Rundeck, Pivotal, Opsgenie docs

  • d602014 integration-docs: Update Radarr doc.
  • f73f5ba integration-docs: Update Raygun doc.
  • 62de077 integration-docs: Update Review Board doc.
  • ec6aeec integration-docs: Update RhodeCode doc.
  • d962875 integration-docs: Update Rundeck doc.
  • c0d2115 integration-docs: Update Pivotal doc.
  • db0ad8a integration-docs: Update Opsgenie doc.

#31012 Removing Solano integration

  • f6e079c integrations: Remove Solano integration.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

🧩 Other Independent Work

#1530 Rename "All Messages" to "Combined Feed"1

This was done as part of a single pair programming session. Thus, the PR needs much more work before it is ready to be merged.

The goal was to lay the groundwork for supporting version-dependent naming, beginning with this specific naming change. However, the PR currently only addresses this single case, and is not generic.

We anticipated a quick task that would take a couple of hours, but it was interesting how it turned out to be more complex than expected, revealing challenges like character count constraints, cyclical imports, additional tests, and various design decisions.


#1495 Add tests for saving drafts1

This PR adds 46 new test cases. The tests are working well, but I've been advised to reduce some of the indirection to simplify the complexity before it is ready to be merged.

This work was an exercise in pytest :)

  • 696eba Add tests for saving drafts. 5 test functions, 5 fixtures and 2 factory functions.
  • 9ab690 Add draft cases for when recipients match saved draft.

#31319 Avoid unintentionally marking messages as unread in interleaved views1

  • b06c5cd Add confirm_dialog to bulk mark unread in interleaved views.

#1492 Adopt urwid_readline for all editors2

The app uses several editors, only one of which was previously an urwid_readline editor, with the added functionality of recognizing GNU Readline shortcuts.

  • 7ad185 Adopt urwid_readline for all editors.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ” Code Reviews

My Experience with Reviewing

Zulip requires contributors to team up and provide "buddy reviews" for their partners.

Initially, I spent a huge chunk of my time on code reviews, even more time than I spent on my own work. I had been thoroughly analysing the problem, the motivation behind the PR, the design decisions, try to figure out how to arrive at a similar solution myself, comparing and evaluating different implementation options. In my mind, before I approved a PR, I needed to be completely confident about every aspect of the PR.

It took me a couple of weeks, exploring other buddy and mentor reviews (and watching Alya Abbott's Live Code Review session and Neil Pilgrim's session shortly later helped too), to realise that my approach was overkill. The actual expectation was to conduct a basic check β€” verifying that the code adheres to general guidelines and testing the functionality for bugs.

It was interesting to learn that it's not even necessary to go through every line of code right away, one can pick and choose what aspects to review based on one's own comfort levels. And Neil Pilgrim explained how the aspects you focus on should vary with the stage of review, as the PR progresses in the review cycle.

This new understanding brought me relief, and I began to actually enjoy the review process. I got to have fun playing around with the new features as a user, explore the PR code as a curious developer, and just comment my thoughts and observations, and bang it's a review.

This experience, although unintentional, also served as a role-play exercise, that deepened my appreciation for the critical responsibility of a repo maintainer, who serves as the last bastion against all bad code before they get merged.


πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ’¬ Discussions Sparked

Apart from the code contributions listed above, here are some additional discussions I was involved in.

πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ πŸ”Έ

πŸ™ Acknowledgements

I am grateful to the Zulip community and Google for giving me this opportunity.

The consistent code reviews and feedback at Zulip are unparalleled, fostering rapid growth by pinpointing areas for improvement. The opportunity to tackle any issues, not limited to my proposal, was a significant benefit. With that degree of freedom, and the vast scope for growth with countless opportunities for impactful contributions, I feel privileged to have been part of such a constructive and healthy work environment at Zulip.

I owe a great deal to Neil Pilgrim, without whom this summer would just not have been possible. Whether it was providing thorough and insightful reviews, offering guidance on complex issues, or sharing technical knowledge through engaging chats, Neil Pilgrim has always been enthusiastic and a strong motivator. I valued the dedication, high standards and meticulous approach he brought to every task, which resonated well with my own principles, and motivated me to uphold them.

Neil Pilgrim's absolute encouragement to come up with our own solutions for the issues, not just the implementations, made the whole experience quite exciting. His amount of patience in explaining why certain approaches were less effective, greatly contributed to my technical and diplomatic skills.

I realize I was a demanding mentee who constantly kept creating new PRs and starting new discussions, and maintained a steady set of PRs that required reviewing. I am extremely thankful to Neil Pilgrim and Moritz Neeb for their tireless efforts and the numerous hours they devoted to reviewing my code. Your commitment and support have been instrumental to my growth.

My heartfelt gratitude to Moritz Neeb for all his supportive kindness. Moritz Neeb’s direct communication style, nurturing manner and approachability were ideal. His pragmatic attitude and groundedness helped balance my detail-oriented nature.

A huge thanks to Lauryn Menard, Tim Abbott and Alya Abbott for their consistent encouragement, patient explanations and swift help in getting all the integration docs PRs merged at such speed. Alya Abbott's prompt for support was key in getting started, her approachability is much appreciated.

My sincere appreciation to Zulip Terminal GSoC team for all the weekly calls.

Greatly appreciate Lauryn Menard and Karl Stolley for hosting the vibrant social discussions, hangouts, and AMAs each week. I’ll miss them greatly. Their support and welcoming nature made me feel at home with Zulip.

Much obliged to Puneeth Chaganti for his guidance in navigating the tech landscape and for sharing his insights and experiences.

A big shout-out to Sashank Ravipati, Pieter CK, Adnan Husain and Afeefuddin for all the buddy reviews, collaborative work, strong team spirit and engaging chats.

✨ Conclusion

This experience has been invaluable in my growth both professionally and personally. I am deeply grateful to Zulip for being an exceptional organization and fostering a thriving developer community that has greatly supported me.

I'd like to stay involved by participating in discussions and code reviews, staying updated on new developments, exploring more projects within Zulip, tackling more complex challenges across the codebase, and supporting new contributors enjoy their experience with Zulip as much as I have.

Thanks to Zulip, this summer has been an extraordinary journey. 🌱

Footnotes

  1. This work hasn't yet been merged. Some of the commit hash links may not work if the PR gets updated. ↩ ↩2 ↩3 ↩4 ↩5 ↩6 ↩7 ↩8 ↩9 ↩10 ↩11 ↩12 ↩13

  2. This work was merged during the community bonding period. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment