Skip to content

Commit

Permalink
code review tutorial
Browse files Browse the repository at this point in the history
main branch for code review tutorial:
3 text exercies
2 python excercises
2 Fortran excercies

issues/ contains the .md files used in the issue bodies
pull_requests/ contains the .md file used in the pull request bodies

workflows - run on demand
  create_exercises - opens the issues and linked pull requests for
                     selected language
  close_issues_and_pulls - closes all open issues and pull requests
  • Loading branch information
hkershaw-brown committed Dec 13, 2023
1 parent 05487a6 commit 3fc9795
Show file tree
Hide file tree
Showing 26 changed files with 693 additions and 0 deletions.
28 changes: 28 additions & 0 deletions .github/workflows/close_issues_and_pulls.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: reset_exercises
on:
workflow_dispatch:
permissions: write-all
jobs:
close-pulls:
runs-on: ubuntu-latest
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v3
- run: |
for p in $(gh pr list --json number --jq '.[].number'); do
gh pr close $p
done
close-issues:
runs-on: ubuntu-latest
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v3
- run: |
for p in $(gh issue list --limit 200 --json number --jq '.[].number'); do
gh issue close $p
done
58 changes: 58 additions & 0 deletions .github/workflows/create_exercises.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
name: create_exercises
on:
workflow_dispatch:
inputs:
language:
description: 'Language'
required: true
default: text
type: choice
options:
- text
- python
- Fortran
permissions: write-all
jobs:
create-labels:
runs-on: ubuntu-latest
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v3
- run: gh label create ${{ inputs.language }} -f
create-pulls-matrix:
needs: create-labels
strategy:
matrix:
exercise: [1, 2, 3]
runs-on: ubuntu-latest
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- run: |
echo "Language: ${{ inputs.language }}"
echo "Exercise: ${{ matrix.exercise }}"
# checkout repo
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: create issue for exercise ${{ inputs.exercise }}
# check if the issue file exists before creating the issue
# store created issue in BAR to use in pull request text
env:
issueFile: 'issues/${{ inputs.language }}-ex${{ matrix.exercise }}-issue.md'
if: hashFiles(env.issueFile) !=''
run: |
echo "BAR=$(gh issue create --title "${{ inputs.language }}: Exercise ${{ matrix.exercise }}" --body-file "issues/${{ inputs.language }}-ex${{ matrix.exercise }}-issue.md" --label ${{ inputs.language }})" >> $GITHUB_ENV
- name: query_branch_exists
run: |
echo "FOO=$(git branch -r | grep "${{ inputs.language }}-${{ matrix.exercise }}")" >> $GITHUB_ENV
- name: create pull request for exercise ${{ matrix.exercise }}
run: |
if [ '${{ env.FOO }}' == '' ]; then
echo "no branch ${{ inputs.language }}-${{ matrix.exercise }} to create pull request from"
else
cat pull_requests/${{ inputs.language }}-ex${{ matrix.exercise }}-pullbody.md > pullbody
echo "Fixes ${{ env.BAR }} ${{ inputs.language }}" >> pullbody
gh pr create -B main -H "${{ inputs.language }}-${{ matrix.exercise }}" --title 'Exercise ${{ matrix.exercise }} ${{ inputs.language }}.' --body-file pullbody --label ${{ inputs.language }}
fi
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
a.out
*.o
*.mod
*.exe
__pycache__/
13 changes: 13 additions & 0 deletions Fortran/exercise1/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
FC=gfortran #fortran compiler
FFLAGS=-O2
SRC=calc_pi.f90
OBJ=${SRC:.f90=.o} #substitute .f90 with .o

%.o: %.f90 #wildcard rule, creation of *.o depends on *.f90
$(FC) $(FFLAGS) -o $@ -c $<

calc_pi.exe: $(OBJ)
$(FC) $(FFLAGS) -o $@ $(OBJ)

clean: #cleans all the old compilation files
@rm -f *.mod *.o *.exe
28 changes: 28 additions & 0 deletions Fortran/exercise1/calc_pi.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
program calc_pi
implicit none

integer, parameter :: num_samples = 1000000 ! Number of random samples

real :: x, y
real :: pi_approx
integer :: i, count_inside_circle

count_inside_circle = 0

! Loop through random points and check if they fall inside the unit circle
do i = 1, num_samples
call random_number(x)
call random_number(y)

if (x**2 + y**2 <= 1.0) then
count_inside_circle = count_inside_circle + 1
end if
end do

! Approximate pi value
pi_approx = 4.0 * real(count_inside_circle) / real(num_samples)

print *, "Approximated value of pi:", pi_approx

end program calc_pi

13 changes: 13 additions & 0 deletions Fortran/exercise2/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
FC=gfortran #fortran compiler
FFLAGS=-O2
SRC=league_sim.f90
OBJ=${SRC:.f90=.o} #substitute .f90 with .o

%.o: %.f90 #wildcard rule, creation of *.o depends on *.f90
$(FC) $(FFLAGS) -o $@ -c $<

league_sim.exe: $(OBJ)
$(FC) $(FFLAGS) -o $@ $(OBJ)

clean: #cleans all the old compilation files
@rm -f *.mod *.o *.exe
95 changes: 95 additions & 0 deletions Fortran/exercise2/league_sim.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
program football_league_simulator
implicit none

integer, parameter :: num_teams = 5
integer :: points(num_teams)
character(len=20) :: team_names(num_teams) = ['Team A', 'Team B', 'Team C', 'Team D', 'Team E']
integer :: i, j

! Initialize points
points = 0
! Simulate matches and update points
do i = 1, num_teams
do j = i+1, num_teams
call simulate_match(i, j, points)
end do
end do

call sort_teams(team_names, points)

! Print results
call print_results(team_names, points)


contains

subroutine simulate_match(team1, team2, points)
implicit none
integer, intent(in) :: team1, team2
integer, intent(inout) :: points(:)

integer :: goals1, goals2
real :: g1, g2

call random_seed()

! Simulate goals for each team (max 5 goals)
call random_number(g1)
call random_number(g2)
goals1 = floor(6*g1)
goals2 = floor(6*g2)

! Update points based on match result
if (goals1 > goals2) then
points(team1) = points(team1) + 3 ! Team 1 wins
else if (goals2 > goals1) then
points(team2) = points(team2) + 3 ! Team 2 wins
else
points(team1) = points(team1) + 1 ! Draw, each team gets 1 point
points(team2) = points(team2) + 1
end if

end subroutine simulate_match

subroutine sort_teams(team_names, points)
implicit none
character(len=20), intent(inout) :: team_names(:)
integer, intent(inout) :: points(:)
integer :: i, j
integer :: tempPoints
character(len=20) :: tempName

do i = 1, size(team_names) - 1
do j = 1, size(team_names) - i
if (points(j) < points(j + 1)) then
! Swap points
tempPoints = points(j)
points(j) = points(j + 1)
points(j + 1) = tempPoints

! Swap team names accordingly
tempName = team_names(j)
team_names(j) = team_names(j + 1)
team_names(j + 1) = tempName
end if
end do
end do
end subroutine sort_teams



subroutine print_results(team_names, points)
implicit none
character(len=20), intent(in) :: team_names(:)
integer, intent(in) :: points(:)
integer :: i


print *, "Football League Results:"
print *, "------------------------"
do i = 1, size(team_names)
print *, team_names(i), ": ", points(i), " points"
end do
end subroutine print_results

end program football_league_simulator
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Welcome to code review tutorial GitHub repository

Vist [code-review.org](https://code-review.org/) to learn more.

This work was supported by the Better Scientific Software Fellowship
Program, funded by the Exascale Computing Project (17-SC-20-SC), a
collaborative effort of the U.S. Department of Energy (DOE) Office of
Science and the National Nuclear Security Administration; and by the
National Science Foundation (NSF) under Grant No. 2154495.
21 changes: 21 additions & 0 deletions issues/Fortran-ex1-issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
## Fortran Exercise 1: Calculating Pi

A program to calculate pi is being used as a sample code to
compare how compiler options affect run time.

However, the program is producing an incorrect value for pi.

Take a look at the code.

* Are there any errors that standout?
* Is the code easy to understand?
* What changes, if any, would you make to the code?


Take a look at the pull request

* Does the pull request address the issue?
* Are there any unnecessary or incorrect changes?
* Why did the author change the precision? Can you find the commit message with the reason?


20 changes: 20 additions & 0 deletions issues/Fortran-ex2-issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
## Fortran Exercise 2: Refactoring

A team member (Betty) has been working on a project to simulate
a football league over a season. Betty's code is all in one
program in a single file, league_sim.f90. However, Betty would now like to use
the routines contained in the main program in several of their other programs.

Betty created a module to enable the routines to be used by other programs and
submitted their changes in a pull request. They'd like you to take a look at their code
and offer suggestions on how to improve it.

Take a look at the pull request:

* What would make the pull request easier to review?
* What would improve the pull request? Add a comment.
* Are the comments up to date, necessary, helpful?
* Would you remove some of these comments? Add suggestions for which comments to remove and/or change.
* Which module variables and routines should be public?
* Add a suggestion for which variables/routines to make private to the module.
* Have any bugs been introduced during the code refactoring?
28 changes: 28 additions & 0 deletions issues/python-ex1-issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## Python Exercise 1: Printing out vowels and consonants

A python script `vowel_counter.py` is supposed to print out the number of vowels and consonants in a string.
Several users have reported problems, and one user has put in a pull request to improve the script.

User reports:

```
Only vowels are getting printed out! This script is not working
```

```
This script is printing the number of consonants.
Why is `vowel_counter.py` even printing out the number of consonants?
```

Take a look at the python script.

* Can you see any errors?
* Do you need to run the script to test the behavior?


Take a look at the pull request.

* Does the pull request address the issue?
* Are the comments up to date, necessary, helpful? Would you remove some of these comments? Add suggestions for which comments to remove and/or change.
* Are there any bugs?
* What would improve the pull request? Could you use list comprehension? Would you except the solution as it is now?
34 changes: 34 additions & 0 deletions issues/python-ex2-issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
## Python Exercise 2: Calculating experiment time windows

A lab group is using a python script to calculate the number of hour-long
experiments between an initial and final time.

Some background: The goal is to run the NSF NCAR space weather model
[TIEGCM](https://www.hao.ucar.edu/modeling/tgcm/tie.php) and stop every
hour to update the model using observations.
Each experiment uses observations from +/- 30 minutes from when the model
is stopped. This +/- 30 minutes is known as the experiment window.

The input to the python script is the intial time, the final time, and the
delta (the duration of a single experiment). The script calculates the number
of experiments needed, and for each experiment caclulates the start and stop
for both the model and the window.

The issue: For one set of experiments, the delta was accidentally entered
as negative. The number of windows was calculated correctly, but the times
for each of the windows were incorrect.

The lab group want to add some error checking to the python script to prevent
this error happening again.

Take a look at the script:

* Where would you put the error check?
* Are there other error checks that you think should be included in the script?

Take a look at the pull request:

* Does the pull request address the issue?
* Are there any unnecessary or incorrect changes?
* What would make the pull request easier to review?
* What would improve the pull request? Add a suggestion.
Loading

0 comments on commit 3fc9795

Please sign in to comment.