Skip to content

Use object with null prototype for settings - #4835

Closed
EvanHahn wants to merge 2 commits into
expressjs:5.xfrom
EvanHahn:ignore-settings-on-object-prototype-5.x
Closed

Use object with null prototype for settings#4835
EvanHahn wants to merge 2 commits into
expressjs:5.xfrom
EvanHahn:ignore-settings-on-object-prototype-5.x

Conversation

@EvanHahn

@EvanHahnEvanHahn commented Feb 21, 2022

Copy link
Copy Markdown
Contributor

express().settings is now an object with a null prototype. This simplifies the code.

Comment threadlib/application.js Outdated
Comment threadtest/app.locals.js Outdated
Comment threadtest/config.js Outdated
@EvanHahn
EvanHahnforce-pushed the ignore-settings-on-object-prototype-5.x branch from 5057a23 to e49bb35CompareFebruary 26, 2022 05:02
@EvanHahn

Copy link
Copy Markdown
ContributorAuthor

Just rebased this.

@EvanHahn

Copy link
Copy Markdown
ContributorAuthor

Anything else I should do here? No rush from me, just wanna make sure I'm not blocking anything.

@dougwilson

Copy link
Copy Markdown
Contributor

Hey! No, these 5.x changes are good. I'm just waiting to merge after the 4.x branch is merged in to reduce the merge conflicts is all. That should be very soon, as the 4.x changes landed are supposed to be out next week! Thank you for your hard work and discovering this issue. API is more sane now thanks to you 😊

@EvanHahn

Copy link
Copy Markdown
ContributorAuthor

Great, thanks!!

If there are other things that need doing, feel free to reach out to me@evanhahn.com and I'll see if I can help.

@EvanHahn

Copy link
Copy Markdown
ContributorAuthor

This has been open for over a year. No rush from me, but let me know if there's anything I can do to move this along.

Comment threadHistory.md Outdated
@EvanHahn
EvanHahnforce-pushed the ignore-settings-on-object-prototype-5.x branch from e49bb35 to 01395fbCompareApril 28, 2024 23:24
@EvanHahn
EvanHahnforce-pushed the ignore-settings-on-object-prototype-5.x branch from 01395fb to 790d083CompareApril 28, 2024 23:25
@EvanHahnEvanHahn changed the title Ignore settings on Object.prototypeUse object with null prototype for settingsApr 28, 2024
@EvanHahn

Copy link
Copy Markdown
ContributorAuthor

It seems that the 5.x branch was rebased against main. That means that this PR is primarily a code cleanup, and should be ready to merge.

@wesleytodd

wesleytodd commented Apr 29, 2024

Copy link
Copy Markdown
Member

#4861 is merged which created a conflict here. If you want to resolve it go for it (its super simple) or I can with a local merge (instead of just clicking the buttons). I will leave it here for a bit and if I don't hear back will merge it when I circle back.

@EvanHahn

Copy link
Copy Markdown
ContributorAuthor

Fixed, I believe.

@wesleytodd

Copy link
Copy Markdown
Member

Hm, it still doesn't like it, but afaict that was the line which changed in both. This is the reason not to leave branches to become stale this long, sorry about the difficulty. I will pull and check it locally now.

@wesleytodd

Copy link
Copy Markdown
Member

🤷 merged in the referenced cherry-picked version above.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@EvanHahn@dougwilson@wesleytodd@UlisesGascon@ejcheng