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

Update frameworks example apps #368

Merged
merged 5 commits into from
Feb 19, 2021
Merged

Update frameworks example apps #368

merged 5 commits into from
Feb 19, 2021

Conversation

bxsx
Copy link
Contributor

@bxsx bxsx commented Feb 15, 2021 •
edited
Loading

Description of the change

Update Flask example to make it simpler and working.
Add note about required packages to the Twisted example

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • Chore (non-breaking change)

Related issues

Fix ch79857

@bxsx
Keep Flask's example app as simple as possible
b40e4cc 
The previous example was unnnecessarily complex and didn't work at all.
It probably should be rewritten as a part of more robust examples (or
advanced docs).
@bxsx
Add note about required additional packages installation
b4d8329
@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #79857: Flask framework reincarnation.

@bxsx
Copy link
Contributor Author

bxsx commented Feb 17, 2021

I added f6547a6 to keep Django compatible across different versions.
It doesn't look great and contain redundant code due to Django's restrictions.

Another option would be to keep 2 examples version-dependent. Thoughts?

@waltjones
Copy link
Contributor

Maybe clean up some of the duplication by setting up the keyword arguments first as a dict. You still have to callsettings.configure conditionally, but the arguments are set up once (replacing/renaming the one argument of course).

@bxsx
Copy link
Contributor Author

bxsx commented Feb 17, 2021

Maybe clean up some of the duplication by setting up the keyword arguments first as a dict. You still have to callsettings.configure conditionally, but the arguments are set up once (replacing/renaming the one argument of course).

Thank @waltjones ! I have added it at 7033c15

@bxsx
Remove a pyrollbar top-module doc-string about Pyramid
8c2f8eb 
This docstring shall be used for general description of pyrollbar and
shouldn't indicate a usage of pyrollbar only with Pyramid.
waltjones
waltjones approved these changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@waltjones waltjones left a comment

Choose a reason for hiding this comment

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

👍 Looks good.

@bxsx bxsx merged commit c6edc30 into master Feb 19, 2021
@bxsx bxsx deleted the bxsx/ch79857/sdk-frameworks branch February 19, 2021 19:39
@bxsx bxsx self-assigned this Mar 8, 2021
@bxsx bxsx added this to the v0.16.0 milestone Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waltjones waltjones waltjones approved these changes

@pawelsz-rb pawelsz-rb pawelsz-rb approved these changes

@mrunalk mrunalk Awaiting requested review from mrunalk

Assignees

@bxsx bxsx

Labels
None yet
Projects
None yet
Milestone
v0.16.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bxsx @waltjones @pawelsz-rb