Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

Commit 5036c01

Browse files
twoGiantsclaude
andcommitted
refactor: consolidate GitHub services
Merged src/services/github/ into src/services/source-control/. The duplicate OctokitGitHubService push logic now lives in GithubService behind a unified SourceControlService interface with a new push() method. Removed the src/services/github/ directory entirely. Renamed BackendFunctionService to FunctionBackendService and useSourceControl to useSourceControlService to follow naming conventions. Removed duplicate @octokit/rest from devDependencies. Extracted useCreateFunctionForm hook inside CreateFunctionForm and useFunctionCreatePage hook inside FunctionCreatePage per architecture guidelines. Removed PAT field from create form (PAT management deferred to upcoming session-wide PAT feature). Updated design doc and workflow with received PR reviews section. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent aa34c2b commit 5036c01

25 files changed

Lines changed: 1519 additions & 256 deletions

docs/STYLEGUIDE.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
- **No `console.log`**: Use structured approach if logging needed.
1919
- **Naming**: `use*` for hooks, `*Service` for services, PascalCase for components/types.
2020

21+
## Documentation
22+
23+
- **No em dashes (``)**. Use commas, periods, or parentheses instead.
24+
2125
## OCP Plugin Styling Constraints
2226

2327
The `.stylelintrc.yaml` enforces strict rules to prevent breaking the OpenShift Console:

docs/WORKFLOW.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ After [Startup Sequence](#startup-sequence), work through the picked feature:
2121
3. **Implement** — using `/executing-plans` skill
2222
4. **Review** — code review using `/requesting-code-review` skill, fix found issues
2323
5. **Manual Test** — use browser automation and validate it works in the browser
24-
6. **Complete** — flip `passes` to `true` in [`docs/features.json`](references/features-json-readme.md), update [`docs/claude-progress.txt`](references/claude-progress-readme.md), commit
24+
6. **Complete** — flip `passes` to `true` in [`docs/features.json`](references/features-json-readme.md), update [`docs/claude-progress.txt`](references/claude-progress-readme.md), move plan to `docs/plans/completed/`, commit
2525
7. **PR** — push branch, open PR per [Pull Requests](#pull-requests) convention
26-
8. Move plan to `docs/plans/completed/`
27-
9. Stop, wait for user command
26+
8. Stop — wait for PR review. Rework per [Received PR Reviews](#received-pr-reviews) when asked.
27+
28+
## Received PR Reviews
29+
30+
For each comment: read the full text and its diff hunk context, make the fix, then re-read the comment and verify your change actually matches what was asked (placement, naming, scope — not just compilation). Reply in the thread stating what changed.
2831

2932
## Branching
3033

@@ -38,6 +41,8 @@ Open PRs via `gh pr create` using the template at `.github/pull_request_template
3841

3942
Types are the same as [conventional commits](references/commit-message-guide.md#conventional-commits) but capitalized.
4043

44+
No em dashes (``) in PR titles or descriptions. Use commas, periods, or parentheses instead.
45+
4146
## Session Rules
4247

4348
- One feature at a time

docs/claude-progress.txt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,25 @@
11
# Claude Progress Log
22
# Newest entries first. Agents: append your entry at the top after the header.
33

4+
---
5+
## 2026-04-16 | Session: PR #5 review cleanup
6+
Worked on: Consolidate duplicate GitHub services, fix naming conventions, extract hooks per architecture guidelines
7+
Completed:
8+
- Merged src/services/github/ push logic into src/services/source-control/GithubService
9+
- Added push() method to SourceControlService interface
10+
- Deleted src/services/github/ directory entirely
11+
- Renamed BackendFunctionService → FunctionBackendService
12+
- Renamed useSourceControl → useSourceControlService
13+
- Extracted useCreateFunctionForm hook from CreateFunctionForm (single fields state object + setField helper)
14+
- Extracted useFunctionCreatePage hook from FunctionCreatePage (rendering-only component)
15+
- Removed duplicate @octokit/rest from devDependencies
16+
- Updated design doc with all renames and FileEntry[] signatures
17+
- Code review: added missing error alert test, fixed quote style, cleaned up GeneratedFiles refs
18+
- Added missing feature entries to features.json (PAT Avatar, namespace listing, PR #5 cleanup)
19+
- 10 suites, 41 tests, all passing
20+
Left off: Branch ready for PR. Manual browser test skipped (no browser in sandbox).
21+
Blockers: None
22+
423
---
524
## 2026-04-02 | Session: Function Create Page
625
Worked on: Full-stack Function Create Page feature (plan 003)

docs/design/2026-03-16-faas-poc-design.md

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ flowchart TB
2020
subgraph UI [Browser - React + PatternFly 6]
2121
VIEWS[Views: List / Create / Editor]
2222
FS[useFunctionService hook]
23-
SCS[useSourceControl hook]
23+
SCS[useSourceControlService hook]
2424
CS[useClusterService hook]
2525
end
2626
@@ -117,15 +117,15 @@ func-console/
117117
118118
├── src/
119119
│ ├── services/ # interfaces + implementations
120-
│ │ ├── types.ts # FunctionConfig, GeneratedFiles, etc.
120+
│ │ ├── types.ts # FunctionConfig, FileEntry, RepoInfo, etc.
121121
│ │ ├── function/
122122
│ │ │ ├── FunctionService.ts # TypeScript interface
123123
│ │ │ ├── useFunctionService.ts # hook returning singleton
124-
│ │ │ └── FunctionService.github.ts # PoC implementation
124+
│ │ │ └── FunctionBackendService.ts # backend proxy implementation
125125
│ │ ├── source-control/
126126
│ │ │ ├── SourceControlService.ts # TypeScript interface
127-
│ │ │ ├── useSourceControl.ts # hook returning singleton
128-
│ │ │ └── SourceControlService.github.ts
127+
│ │ │ ├── useSourceControlService.ts # hook returning singleton
128+
│ │ │ └── GithubService.ts # Octokit implementation (list, fetch, push)
129129
│ │ └── cluster/
130130
│ │ ├── ClusterService.ts # TypeScript interface
131131
│ │ └── useClusterService.ts # hook wrapping OCP SDK hooks
@@ -174,7 +174,6 @@ interface FileEntry {
174174
type: 'blob';
175175
}
176176

177-
type GeneratedFiles = Map<string, FileEntry>; // filepath → content
178177
```
179178

180179
### FunctionService
@@ -195,8 +194,7 @@ interface FunctionService {
195194
}
196195
```
197196

198-
- `generateFunction` → func.yaml, handler code, package files, .gitignore, tests
199-
- `generateWorkflow` → .github/workflows/func-deploy.yaml
197+
- `generateFunction` → func.yaml, handler code, package files, .gitignore, tests, github workflow
200198
- Output consumed by TreeView+CodeEditor (display) and Octokit (push)
201199

202200
#### Error Handling
@@ -224,8 +222,8 @@ interface SourceControlService {
224222
isAuthenticated(): boolean;
225223
createRepo(name: string): Promise<RepoInfo>;
226224
listFunctionRepos(): Promise<RepoInfo[]>;
227-
push(repo: RepoInfo, files: GeneratedFiles, message: string): Promise<void>;
228-
fetch(repo: RepoInfo): Promise<GeneratedFiles>;
225+
push(repo: RepoInfo, files: FileEntry[], message: string): Promise<void>;
226+
fetch(repo: RepoInfo): Promise<FileEntry[]>;
229227
createSecret(repo: RepoInfo, name: string, value: string): Promise<void>;
230228
createVariable(repo: RepoInfo, name: string, value: string): Promise<void>;
231229
}
@@ -338,7 +336,7 @@ The Functions List Page needs data from **both** services. A dedicated merging h
338336
```tsx
339337
// useFunctionsList: merges GitHub repos (source of truth) with cluster status
340338
export function useFunctionsList(): { functions: FunctionListItem[]; loaded: boolean } {
341-
const { repos, loaded: reposLoaded } = useSourceControl();
339+
const { repos, loaded: reposLoaded } = useSourceControlService();
342340
const { functions: deployments, loaded: deploymentsLoaded } = useClusterService();
343341

344342
const functions = mergeFunctionData(repos, deployments);
@@ -435,7 +433,7 @@ Form validation errors shown inline. Background errors (repo creation, push) sur
435433

436434
### Functions Edit Page
437435

438-
PatternFly TreeView sidebar + SDK CodeEditor. Tree built from `GeneratedFiles` map keys split on `/`. Shows full repo contents.
436+
PatternFly TreeView sidebar + SDK CodeEditor. Tree built from `FileEntry[]` paths split on `/`. Shows full repo contents.
439437

440438
**Actions:**
441439

@@ -447,7 +445,7 @@ PatternFly TreeView sidebar + SDK CodeEditor. Tree built from `GeneratedFiles` m
447445
| Component | Purpose |
448446
|-----------|---------|
449447
| `CodeEditor` | Monaco-based code editor (lazy loaded) |
450-
| PatternFly TreeView | File tree sidebar built from `GeneratedFiles` map keys |
448+
| PatternFly TreeView | File tree sidebar built from `FileEntry[]` paths |
451449
| `ErrorBoundaryFallbackPage` | Catch unexpected errors |
452450

453451
#### Error Handling

docs/features.json

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,49 @@
8181
"VM isolation ensures no leakage to host workstation"
8282
],
8383
"passes": false
84+
},
85+
{
86+
"category": "functional",
87+
"description": "GitHub PAT Avatar: session-wide PAT entry via modal + avatar component showing authenticated GitHub user",
88+
"steps": [
89+
"PatModal opens automatically on first visit to Function List Page when no PAT is stored in session",
90+
"PatModal is dismissable — user can close it without entering a PAT",
91+
"PatModal validates the PAT against GitHub API and on success stores it in session and closes",
92+
"SourceControlService interface extended with init(pat: string) and isInitialized(): boolean methods; GithubService implementation updated accordingly",
93+
"UserAvatar component renders inside ListPageHeader at the top-right corner on every page (list, create, edit)",
94+
"UserAvatar has two states: not initialized shows 'Connect to GitHub' with a key/lock icon (clickable, opens PatModal); initialized shows GitHub username with avatar icon",
95+
"UserAvatar is clickable only on the Function List Page, clicking reopens PatModal to change the PAT and associated GitHub user",
96+
"On Create and Edit pages, UserAvatar displays the GitHub user but is not clickable (PAT cannot be changed from those pages)",
97+
"When PatModal is dismissed without a PAT, the Function List Page (both empty state and table view) shows a hint text directing the user to click 'Connect to GitHub' in the top-right corner to set a PAT",
98+
"PAT change logic is encapsulated in a useUserAvatar custom hook following layered architecture (Hook imports Service, Component imports Hook)"
99+
],
100+
"passes": false
101+
},
102+
{
103+
"category": "functional",
104+
"description": "Function List Page shows deployed functions from all user-accessible namespaces without requiring a PAT",
105+
"steps": [
106+
"ClusterService refactored: remove ALL_NAMESPACES and useActiveNamespace logic, always watch across all user-accessible namespaces (functions in GitHub repos can target different namespaces via func.yaml, so single-namespace view is not useful; filtering deferred to a later feature)",
107+
"ClusterService lists all namespaces/projects accessible to the logged-in user and watches Deployments with label function.knative.dev/name across all of them",
108+
"Function List Page renders deployed functions even when no PAT has been entered (PatModal dismissed or not yet shown)",
109+
"When PAT is later provided, repo data from SourceControlService is merged with already-visible deployed functions",
110+
"Deployed functions without a matching repo still appear in the table with available cluster data (name, namespace, status, replicas, url)"
111+
],
112+
"passes": false
113+
},
114+
{
115+
"category": "technical",
116+
"description": "PR #5 review cleanup: consolidate duplicate GitHub services, fix naming conventions, extract hooks per architecture guidelines",
117+
"steps": [
118+
"Merge src/services/github/ into src/services/source-control/, combine OctokitGitHubService push logic into GithubService, remove src/services/github/ directory entirely",
119+
"Rename GitHubService interface to SourceControlService and add push method to the existing SourceControlService interface (rename pushFiles to push per design doc)",
120+
"Rename useGitHubService hook to useSourceControlService",
121+
"Rename BackendFunctionService.ts to FunctionBackendService.ts",
122+
"Remove duplicate @octokit/rest from devDependencies in package.json (keep only the entry under dependencies)",
123+
"Extract useFunctionCreatePage hook from FunctionCreatePage — move service calls into useEffect (both FunctionService and SourceControlService call external APIs), keep the component rendering-only",
124+
"Extract useCreateFunctionForm hook from CreateFunctionForm — consolidate individual useState calls into a single fields state object with a setField(key, value) helper, derive isValid from fields, remove handleSubmit (call onSubmit(fields) directly)",
125+
"Update design doc (2026-03-16-faas-poc-design.md) to reflect all renames and new method signatures"
126+
],
127+
"passes": true
84128
}
85129
]

0 commit comments

Comments
 (0)