8000 Rule out future bad behaviour using admin overrides. by fnzprac · Pull Request #5 · practiv/.github · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Rule out future bad behaviour using admin overrides. #5

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rule out future bad behaviour using admin overrides. #5

wants to merge 1 commit into from

Conversation

fnzprac
Copy link
Contributor
@fnzprac fnzprac commented Oct 26, 2021

Change description

Changes were just made for a specific customer in such a way that they impact all customers - this is wholly unacceptable and needs to stop happening, especially where there is already a mechanism in place to make it possible without such a change. Add 2 new check boxes to at least force client-side-consultants to think twice before they do something that sets us back and doesn't push us forward.

Type of change

  • Bug fix (fixes an issue)
  • New feature (adds functionality)
  • Chore (maintenance, clean up, etc)
  • Documentation (README files, pom descriptions, etc)
  • Style (formatting, whitespace, indentation, etc)
  • Refactor (exclusively no functional change)
  • Performance (exclusively, no functional change)
  • Tests (No change to working codebase, only testing around it)

Related issues/PRs and references

N/A

Checklists

Development

  • branchout mvn cv passes locally
  • Major version bumped if breaking change
  • Lint rules pass locally
  • Application changes have been tested thoroughly
  • Automated tests covering modified code pass

Code best practices

  • Global exception handling considered
  • Log messages are relevant

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

Standards

  • Commit comments are appropriate?
  • Change notes included?

Pre code review (for PR creator)

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached as necessary
  • "Ready for review" label attached and reviewers assigned
  • Github issue is referenced in the Pull Request or commit
  • Commits are atomic and self contained
  • Branch is complete and overall change functionally usable
  • Committer and author name and email are correct/appropriate, not git defaults
  • Branch contains no breaking changes or major version is bumped. (TODO link) Or did something break that you need to isolate? (TODO link)

I promise not to...

  • Force push after review is done - instead add new commits with comments prefixed with "SQUASH" or "FIXUP" that can be git rebase -i'd later.
  • Resolve conversations on behalf of the person who raised them - that's for them to do after you satisfy them or fix the issue in a new commit.

PR Template Contributions

Contribute via PR here: https://github.com/practiv/.github

@fnzprac fnzprac requested a review from jonathanptew October 26, 2021 01:23
stickycode
stickycode requested changes Oct 26, 2021
View reviewed changes
Copy link
@stickycode stickycode left a comment

Choose a reason for hiding this comment

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

this whole list needs to be smaller not longer and you are missing the point

@fnzprac fnzprac requested a review from stickycode October 26, 2021 02:34
@fnzprac
Copy link
Contributor Author
fnzprac commented Oct 26, 2021

@stickycode We all agree, see thread on how to fix that - it'll be split into different type PRs and you choose the one that suits and get only questions that are relevant to that type of project/change. But until that, you clearly need to see those two questions to avoid doing silly things.

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

@jonathanptew jonathanptew Awaiting requested review from jonathanptew

@stickycode stickycode Awaiting requested review from stickycode

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fnzprac @stickycode
0