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

Remove "Error caused by hoks-id ..." log print #561

Merged
merged 1 commit into from
Nov 21, 2023
Merged

Remove "Error caused by hoks-id ..." log print #561

merged 1 commit into from
Nov 21, 2023

Conversation

severij
Copy link
Contributor

@severij severij commented Nov 21, 2023 •
edited
Loading

Kuvaus muutoksista

Tässä siis poistettu tuo otsikonmukainen lokitus change-hoks!-funktiosta. Lokitusta ei siis ole lisätty, toisin kuin tiketin EH-1377 kuvauksessa on ehdotettu. Tuo tiketti on muutenkin tosi vanha, ja aika paljon refaktorointia on ehtinyt tapahtua koodissa sen jälkeen. Tiketin otsikko on myös varsin lavea, tämä on tottakai moka jonka olen itse aikoinaan tehnyt. Tuon lokituksen poistaminen tuntui tässä tapauksessa sopivalta, koska käsittelemättömät poikkeukset lokitetaan joka tapauksessa oph/ehoks/common/api.clj poikkeuksen käsittelijässä exception-handler.

https://jira.eduuni.fi/browse/EH-1377

Muistilista PR:n tekijälle ja katselmoijille

Ennen asettamista katselmointiin

  • Build onnistuu ilman virheitä
  • Toiminnallisuuden kattavat yksikkötestit on tehty osana PR:ia
  • PR:n sisältämät muutokset noudattavat sovittuja koodikäytänteitä
  • Koodi on riittävästi dokumentoitu tai se on muuten yksiselitteistä
  • Nimet (muuttujat, funktiot, ...) kuvaavat koodia hyvin

❗ Katselmoijat tarkastavat, että yllä mainitut kohdat toteutuvat

Ennen mergeämistä master-haaralle

  • Vähintään yksi kehittäjä on katselmoinut ja hyväksynyt muutokset
    • Jos muutoksilla voi jotain rikkoessaan olla kauaskantoiset vaikutukset, kannattaa muutokset hyväksyttää useammalla katselmoijalla
  • Katselmoijien esittämät muutosehdotukset on huomioitu
  • Muutokset on testattu QA-ympäristössä
  • Yli jääneet kehityskohteet on tiketöity

@severij
Remove "Error caused by hoks-id ..." log print
8cb09ff 
Removed 'Error caused by hoks-id ...' log print in `change-hoks!` because it
doesn't add any meaningful information and is currently logged on disallowed
updates (Bad Request), which are not erroneous situations from system POV.
@severij severij marked this pull request as ready for review November 21, 2023 16:13
@severij
Copy link
Contributor Author

severij commented Nov 21, 2023

Tää ei mahda vaatia katselmointia 🙂

@severij severij merged commit dfca122 into master Nov 21, 2023
1 check passed
@severij severij deleted the severij/EH-1377 branch November 21, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@severij