Skip to content

86eyt6teg: traffic signal job - #758

Open
TibyanSaad wants to merge 1 commit into
mainfrom
feature/86eyt6teg-traffic-signal-job
Open

86eyt6teg: traffic signal job#758
TibyanSaad wants to merge 1 commit into
mainfrom
feature/86eyt6teg-traffic-signal-job

Conversation

@TibyanSaad

Copy link
Copy Markdown
Collaborator
  • created a traffic signal table
  • added a change in the signal status red ->yellow-> green -> red(again)
  • added a trigger to start the change
  • added a procedure to check the current state of a specific signal

@github-actions

Copy link
Copy Markdown

Here are the code review comments for the pull request:

Code Review Comments

  • Title Issue: The title "86eyt6teg: traffic signal job" should start with the correct ClickUp ticket ID format. Please change it to "86eyt6teg: [Actual Title Here]".
  • Description Improvement: The PR description should explicitly state the changes made, like adding procedures, updating states, and creating triggers. Revise the bullet points to clearly reflect the modifications.
  • Indentation: Indentation should be consistently 4 spaces. Line 8 is not indented correctly.
  • Blank Lines: There are several unnecessary blank lines throughout the code. Please remove them for cleaner formatting (lines 2, 11, 31, 59, 63, 78, 92).
  • Logical Errors:
    • Typo in the comment on line 24: "Chceking" should be "Checking".
    • On line 39, the variable v_state is assigned VARCHAR2 but should include length (e.g., VARCHAR2(10)) for clarity and consistency.
  • Trigger Initialization: In the trigger, make sure the procedure start_signal_watch is called in a way that ensures it won't interfere with the main workflow. There's a placeholder ("TO DO") on line 88, which should be replaced with an appropriate message or function.

Please address these issues to improve the code quality and compliance with project standards.

New%20session%20-%202026-08-31T08%3A01%3A29.020Z
opencode session  |  github run

signal_id NUMBER GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
signal_name VARCHAR2(100) NOT NULL,
state VARCHAR2(10) DEFAULT 'RED' NOT NULL
CONSTRAINT ck_signal_state CHECK (state IN ('RED','YELLOW','GREEN')),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

any specific reason for adding a contaraint here ?

v_next_state := 'RED'; v_required_seconds := 10;
END IF;

IF systimestamp >= v_last_changed_at + NUMTODSINTERVAL(v_required_seconds, 'SECOND') THEN

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

indentation ?

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.

4 participants