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.

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

Unify method naming #252 #320

Closed

Conversation

anjum-qureshi
Copy link

@anjum-qureshi anjum-qureshi commented Mar 2, 2023

Renamed few methods in Match, Cookie.

  1. Index => Value
  2. Name => NamedValue
  3. Values => HasValues
  4. HasMaxAge => ContainsMaxAge

Hey @gavv please review the code changes

@gavv
Copy link
Owner

gavv commented Mar 2, 2023

Hi, thanks, but that issue was not actually marked as help-wanted (and is already assigned).

These renames will be done slowly during several releases, to give users time to update.

Also, as described in the issue, renames will be done via adding a new method and deprecating the old one. Just renaming a method, as you did here, will break compatibility.

@gavv gavv closed this Mar 2, 2023
@anjum-qureshi
Copy link
Author

@gavv Apologies I wasn't aware of it. Do we have a contributing docs for the repo Where I can understand the process for contribution/

@gavv
Copy link
Owner

gavv commented Mar 3, 2023

Yes, see "Contributing" section in README. It points to "HACKING.md", which has "Working on a task" section.

General hints for any github repo are:

  • look for issues labeled with "help wanted" and "good first issue"
  • ensure they are not assigned
  • leave a comment before start working to ensure that issue is still relevant and nobody already works on it

@anjum-qureshi anjum-qureshi deleted the 252-unify-method-naming-dev branch March 3, 2023 11:12
@gavv gavv mentioned this pull request Apr 25, 2023
@gavv
Copy link
Owner

gavv commented Apr 25, 2023

#252

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants